diff --git a/NEWS b/NEWS index 2fe8d2a569..13f05baa3b 100644 --- a/NEWS +++ b/NEWS @@ -66,6 +66,8 @@ New Functionality - Add support for SMB filenames in the intel framework. +- Added a new event for weirdness found via file analysis: ``file_weird``. + Changed Functionality --------------------- @@ -103,6 +105,50 @@ Changed Functionality - The Broker C++ API has some breaking changes, see it's own NEWS file for details on how to migrate old code. +- Some Weirds associated with generic binpac parsing exceptions in analyzers + that didn't otherwise handle them (like syslog, modbus, dnp3) are now + a ProtocolViolation instead + +- Weird names that contained variable content and may result in an unbounded + number of weird names have been renamed to remove the variable content + (which has been made available in the "addl" field of conn_weirds): + + - "unknown_dce_rpc_auth_type_%d" -> unknown_dce_rpc_auth_type + - "gtp_invalid_info_element_%d" -> gtp_invalid_info_element + - "unknown_netbios_type:" 0x%x -> unknown_netbios_type + - "excess_netbios_hdr_len" (%d > %d) -> excess_netbios_hdr_len + - "deficit_netbios_hdr_len" (%d > %d) -> deficit_netbios_hdr_len + - "bad_RPC_program (%d)" -> bad_RPC_program + - "unknown_MOUNT_request(%u)" -> unknown_MOUNT_request + - "unknown_NFS_request(%u)" -> unknown_NFS_request + - "RPC resync: discard %d bytes\n" -> RPC_resync + - "RPC_message_too_long (%d64)" -> RPC_message_too_long + - "socks5_unsupported_authentication_method_%d" -> socks5_unsupported_authentication_method + - "socks5_unsupported_authentication_%d_%d" -> socks5_unsupported_authentication + - "ssh_unknown_kex_algorithm=%s" -> ssh_unknown_kex_algorithm + - "Encountered unknown type in server name ssl extension: %d" -> ssl_ext_unknown_server_name_type + - "UDP_datagram_length_mismatch(%d!=%d)" -> UDP_datagram_length_mismatch + - "OPENSSL Could not parse OCSP request (fuid %s)" -> openssl_ocsp_request_parse_error + - "OPENSSL Could not parse OCSP response (fuid %s)" -> openssl_ocsp_response_parse_error + - "Could not parse X509 certificate (fuid %s)" -> x509_cert_parse_error + - "Certificate with invalid BasicConstraint. fuid %s" -> x509_invalid_basic_constraint + - "Could not parse subject alternative names. fuid %s" -> x509_san_parse_error + - "DNS-field does not contain an IA5String. fuid %s" -> x509_san_non_string + - "Weird IP address length %d in subject alternative name. fuid %s" -> x509_san_ip_length + - "Could not parse time in X509 certificate (fuid %s) -- UTCTime has wrong length" -> x509_utc_length + - "Could not parse UTC time in non-YY-format in X509 certificate (x509 %s)" -> x509_utc_format + - "Could not parse time in X509 certificate (fuid %s) -- Generalized time has wrong length" -> x509_gen_time_length + - "Invalid time type in X509 certificate (fuid %s)" -> x509_invalid_time_type + - "Could not parse time in X509 certificate (fuid %s) -- additional char after time" -> x509_time_add_char + - "Could not parse time in X509 certificate (fuid %s) -- not enough bytes remaining for offset" -> x509_time_offset_underflow + - "Could not parse time in X509 certificate (fuid %s) -- unknown offset type" -> x509_time_offset_type + - "X509::GetExtensionFromBIO: %s" -> x509_get_ext_from_bio + - "unknown_mobility_type_%d" -> unknown_mobility_type + - "unknown_routing_type_%d" -> unknown_routing_type + - "unknown_protocol_%d" -> unknown_protocol + - "unknown_gre_version_%d" -> unknown_gre_version + - "unknown_gre_protocol_%u16" -> unknown_gre_protocol + Removed Functionality --------------------- diff --git a/doc b/doc index e404fc80c5..a2c1072f68 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit e404fc80c5c4ecfd0c4441b6b83826761bd985e9 +Subproject commit a2c1072f687067f2e3ef8bfd3d72743576dd4995 diff --git a/scripts/base/frameworks/notice/weird.bro b/scripts/base/frameworks/notice/weird.bro index c6f3748a46..c7a1f3aefb 100644 --- a/scripts/base/frameworks/notice/weird.bro +++ b/scripts/base/frameworks/notice/weird.bro @@ -422,3 +422,13 @@ event net_weird(name: string) local i = Info($ts=network_time(), $name=name); weird(i); } + +event file_weird(name: string, f: fa_file, addl: string) + { + local i = Info($ts=network_time(), $name=name, $addl=f$id); + + if ( addl != "" ) + i$addl += fmt(": %s", addl); + + weird(i); + } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f3dfd42d85..7aa750ac80 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -319,6 +319,7 @@ set(bro_SRCS UID.cc Val.cc Var.cc + WeirdState.cc bsd-getopt-long.c bro_inet_ntop.c cq.c diff --git a/src/Conn.cc b/src/Conn.cc index b55b1210b0..03ecf32703 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -1075,27 +1075,5 @@ void Connection::CheckFlowLabel(bool is_orig, uint32 flow_label) bool Connection::PermitWeird(const char* name, uint64 threshold, uint64 rate, double duration) { - auto& state = weird_state[name]; - ++state.count; - - if ( state.count <= threshold ) - return true; - - if ( state.count == threshold + 1) - state.sampling_start_time = network_time; - else - { - if ( network_time > state.sampling_start_time + duration ) - { - state.sampling_start_time = 0; - state.count = 1; - return true; - } - } - - auto num_above_threshold = state.count - threshold; - if ( rate ) - return num_above_threshold % rate == 0; - else - return false; + return ::PermitWeird(weird_state, name, threshold, rate, duration); } diff --git a/src/Conn.h b/src/Conn.h index ae639d6341..e49314968a 100644 --- a/src/Conn.h +++ b/src/Conn.h @@ -17,6 +17,7 @@ #include "IPAddr.h" #include "TunnelEncapsulation.h" #include "UID.h" +#include "WeirdState.h" #include "analyzer/Tag.h" #include "analyzer/Analyzer.h" @@ -345,14 +346,7 @@ protected: analyzer::pia::PIA* primary_PIA; Bro::UID uid; // Globally unique connection ID. - - struct WeirdState { - WeirdState() { count = 0; sampling_start_time = 0; } - uint64 count = 0; - double sampling_start_time = 0; - }; - - std::unordered_map weird_state; + WeirdStateMap weird_state; }; class ConnectionTimer : public Timer { diff --git a/src/IP.cc b/src/IP.cc index 689876339b..516be7fdd4 100644 --- a/src/IP.cc +++ b/src/IP.cc @@ -288,7 +288,7 @@ RecordVal* IPv6_Hdr::BuildRecordVal(VectorVal* chain) const } default: - reporter->Weird(fmt("unknown_mobility_type_%d", mob->ip6mob_type)); + reporter->Weird(fmt("unknown_mobility_type", mob->ip6mob_type)); break; } @@ -553,7 +553,7 @@ void IPv6_Hdr_Chain::ProcessRoutingHeader(const struct ip6_rthdr* r, uint16 len) #endif default: - reporter->Weird(fmt("unknown_routing_type_%d", r->ip6r_type)); + reporter->Weird(SrcAddr(), DstAddr(), "unknown_routing_type"); break; } } diff --git a/src/Reporter.cc b/src/Reporter.cc index ba1196de21..413f89b9ea 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -13,6 +13,7 @@ #include "Timer.h" #include "plugin/Plugin.h" #include "plugin/Manager.h" +#include "file_analysis/File.h" #ifdef SYSLOG_INT extern "C" { @@ -213,12 +214,14 @@ void Reporter::Syslog(const char* fmt, ...) va_end(ap); } -void Reporter::WeirdHelper(EventHandlerPtr event, Val* conn_val, const char* addl, const char* fmt_name, ...) +void Reporter::WeirdHelper(EventHandlerPtr event, Val* conn_val, file_analysis::File* f, const char* addl, const char* fmt_name, ...) { val_list* vl = new val_list(1); if ( conn_val ) vl->append(conn_val); + else if ( f ) + vl->append(f->GetVal()->Ref()); if ( addl ) vl->append(new StringVal(addl)); @@ -339,7 +342,21 @@ void Reporter::Weird(const char* name) return; } - WeirdHelper(net_weird, 0, 0, "%s", name); + WeirdHelper(net_weird, 0, 0, 0, "%s", name); + } + +void Reporter::Weird(file_analysis::File* f, const char* name, const char* addl) + { + UpdateWeirdStats(name); + + if ( ! WeirdOnSamplingWhiteList(name) ) + { + if ( ! f->PermitWeird(name, weird_sampling_threshold, + weird_sampling_rate, weird_sampling_duration) ) + return; + } + + WeirdHelper(file_weird, 0, f, addl, "%s", name); } void Reporter::Weird(Connection* conn, const char* name, const char* addl) @@ -353,7 +370,7 @@ void Reporter::Weird(Connection* conn, const char* name, const char* addl) return; } - WeirdHelper(conn_weird, conn->BuildConnVal(), addl, "%s", name); + WeirdHelper(conn_weird, conn->BuildConnVal(), 0, addl, "%s", name); } void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name) diff --git a/src/Reporter.h b/src/Reporter.h index bd029c0b59..88270a9dba 100644 --- a/src/Reporter.h +++ b/src/Reporter.h @@ -17,6 +17,7 @@ #include "IPAddr.h" namespace analyzer { class Analyzer; } +namespace file_analysis { class File; } class Connection; class Location; class Reporter; @@ -84,6 +85,7 @@ public: // Report a traffic weirdness, i.e., an unexpected protocol situation // that may lead to incorrectly processing a connnection. void Weird(const char* name); // Raises net_weird(). + void Weird(file_analysis::File* f, const char* name, const char* addl = ""); // Raises file_weird(). void Weird(Connection* conn, const char* name, const char* addl = ""); // Raises conn_weird(). void Weird(const IPAddr& orig, const IPAddr& resp, const char* name); // Raises flow_weird(). @@ -238,7 +240,7 @@ private: // The order if addl, name needs to be like that since fmt_name can // contain format specifiers - void WeirdHelper(EventHandlerPtr event, Val* conn_val, const char* addl, const char* fmt_name, ...) __attribute__((format(printf, 5, 6)));; + void WeirdHelper(EventHandlerPtr event, Val* conn_val, file_analysis::File* f, const char* addl, const char* fmt_name, ...) __attribute__((format(printf, 6, 7)));; void WeirdFlowHelper(const IPAddr& orig, const IPAddr& resp, const char* fmt_name, ...) __attribute__((format(printf, 4, 5)));; void UpdateWeirdStats(const char* name); inline bool WeirdOnSamplingWhiteList(const char* name) diff --git a/src/Sessions.cc b/src/Sessions.cc index 5709ad5f28..edccb7e00c 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -537,8 +537,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr if ( gre_version != 0 && gre_version != 1 ) { - Weird(fmt("unknown_gre_version_%d", gre_version), ip_hdr, - encapsulation); + Weird("unknown_gre_version", ip_hdr, encapsulation); return; } @@ -613,8 +612,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr else { // Not IPv4/IPv6 payload. - Weird(fmt("unknown_gre_protocol_%" PRIu16, proto_typ), ip_hdr, - encapsulation); + Weird("unknown_gre_protocol", ip_hdr, encapsulation); return; } @@ -747,7 +745,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr } default: - Weird(fmt("unknown_protocol_%d", proto), pkt, encapsulation); + Weird("unknown_protocol", pkt, encapsulation); return; } diff --git a/src/WeirdState.cc b/src/WeirdState.cc new file mode 100644 index 0000000000..1f1407a1d2 --- /dev/null +++ b/src/WeirdState.cc @@ -0,0 +1,30 @@ +#include "WeirdState.h" +#include "Net.h" + +bool PermitWeird(WeirdStateMap& wsm, const char* name, uint64_t threshold, + uint64_t rate, double duration) + { + auto& state = wsm[name]; + ++state.count; + + if ( state.count <= threshold ) + return true; + + if ( state.count == threshold + 1) + state.sampling_start_time = network_time; + else + { + if ( network_time > state.sampling_start_time + duration ) + { + state.sampling_start_time = 0; + state.count = 1; + return true; + } + } + + auto num_above_threshold = state.count - threshold; + if ( rate ) + return num_above_threshold % rate == 0; + else + return false; + } diff --git a/src/WeirdState.h b/src/WeirdState.h new file mode 100644 index 0000000000..64dc2bb4b2 --- /dev/null +++ b/src/WeirdState.h @@ -0,0 +1,21 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#ifndef WEIRDSTATE_H +#define WEIRDSTATE_H + +#include +#include + +struct WeirdState { + WeirdState() { count = 0; sampling_start_time = 0; } + uint64_t count = 0; + double sampling_start_time = 0; +}; + +using WeirdStateMap = std::unordered_map; + +bool PermitWeird(WeirdStateMap& wsm, const char* name, uint64_t threshold, + uint64_t rate, double duration); + +#endif // WEIRDSTATE_H + diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index 1fe0dc82bf..818dd917e8 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -223,7 +223,7 @@ void Analyzer::NextPacket(int len, const u_char* data, bool is_orig, uint64 seq, } catch ( binpac::Exception const &e ) { - Weird(e.c_msg()); + ProtocolViolation(fmt("Binpac exception: %s", e.c_msg())); } } } @@ -246,7 +246,7 @@ void Analyzer::NextStream(int len, const u_char* data, bool is_orig) } catch ( binpac::Exception const &e ) { - Weird(e.c_msg()); + ProtocolViolation(fmt("Binpac exception: %s", e.c_msg())); } } } @@ -269,7 +269,7 @@ void Analyzer::NextUndelivered(uint64 seq, int len, bool is_orig) } catch ( binpac::Exception const &e ) { - Weird(e.c_msg()); + ProtocolViolation(fmt("Binpac exception: %s", e.c_msg())); } } } diff --git a/src/analyzer/protocol/bittorrent/BitTorrent.cc b/src/analyzer/protocol/bittorrent/BitTorrent.cc index fd2d5fa914..652d3d120c 100644 --- a/src/analyzer/protocol/bittorrent/BitTorrent.cc +++ b/src/analyzer/protocol/bittorrent/BitTorrent.cc @@ -126,6 +126,4 @@ void BitTorrent_Analyzer::DeliverWeird(const char* msg, bool orig) vl->append(new StringVal(msg)); ConnectionEvent(bittorrent_peer_weird, vl); } - else - Weird(msg); } diff --git a/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc b/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc index 0a3cda37fd..54cac790fb 100644 --- a/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc +++ b/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc @@ -253,8 +253,6 @@ void BitTorrentTracker_Analyzer::DeliverWeird(const char* msg, bool orig) vl->append(new StringVal(msg)); ConnectionEvent(bt_tracker_weird, vl); } - else - Weird(msg); } bool BitTorrentTracker_Analyzer::ParseRequest(char* line) @@ -326,8 +324,11 @@ bool BitTorrentTracker_Analyzer::ParseRequest(char* line) case BTT_REQ_DONE: if ( *line ) - DeliverWeird(fmt("Got post request data: %s\n", line), - true); + { + auto msg = fmt("Got post request data: %s\n", line); + Weird("bittorrent_tracker_data_post_request", msg); + DeliverWeird(msg, true); + } break; default: diff --git a/src/analyzer/protocol/bittorrent/bittorrent-analyzer.pac b/src/analyzer/protocol/bittorrent/bittorrent-analyzer.pac index c3ba226908..232f4a9bd1 100644 --- a/src/analyzer/protocol/bittorrent/bittorrent-analyzer.pac +++ b/src/analyzer/protocol/bittorrent/bittorrent-analyzer.pac @@ -40,7 +40,6 @@ flow BitTorrent_Flow(is_orig: bool) { if ( pstrlen != 19 || memcmp("BitTorrent protocol", pstr.begin(), 19) ) { - connection()->bro_analyzer()->Weird(fmt("BitTorrent: invalid handshake (pstrlen: %hhu, pstr: %.*s)", pstrlen, 19, pstr.begin())); throw Exception("invalid handshake"); } diff --git a/src/analyzer/protocol/dce-rpc/dce_rpc-auth.pac b/src/analyzer/protocol/dce-rpc/dce_rpc-auth.pac index d776f6fec2..44648e35f0 100644 --- a/src/analyzer/protocol/dce-rpc/dce_rpc-auth.pac +++ b/src/analyzer/protocol/dce-rpc/dce_rpc-auth.pac @@ -43,7 +43,7 @@ refine connection DCE_RPC_Conn += { ntlm->DeliverStream(${auth.blob}.length(), ${auth.blob}.begin(), is_orig); break; default: - bro_analyzer()->Weird(fmt("unknown_dce_rpc_auth_type_%d",${auth.type})); + bro_analyzer()->Weird("unknown_dce_rpc_auth_type", fmt("%d", ${auth.type})); break; } diff --git a/src/analyzer/protocol/gtpv1/gtpv1-analyzer.pac b/src/analyzer/protocol/gtpv1/gtpv1-analyzer.pac index 37b7cee0b1..6cf9439363 100644 --- a/src/analyzer/protocol/gtpv1/gtpv1-analyzer.pac +++ b/src/analyzer/protocol/gtpv1/gtpv1-analyzer.pac @@ -319,7 +319,7 @@ void CreatePDP_Request(const BroAnalyzer& a, const GTPv1_Header* pdu) rv->Assign(21, BuildPrivateExt(ie)); break; default: - a->Weird(fmt("gtp_invalid_info_element_%d", (*v)[i]->type())); + a->Weird("gtp_invalid_info_element", fmt("%d", (*v)[i]->type())); break; } } @@ -388,7 +388,7 @@ void CreatePDP_Response(const BroAnalyzer& a, const GTPv1_Header* pdu) rv->Assign(12, BuildPrivateExt(ie)); break; default: - a->Weird(fmt("gtp_invalid_info_element_%d", (*v)[i]->type())); + a->Weird("gtp_invalid_info_element", fmt("%d", (*v)[i]->type())); break; } } @@ -466,7 +466,7 @@ void UpdatePDP_Request(const BroAnalyzer& a, const GTPv1_Header* pdu) rv->Assign(15, BuildEndUserAddr(ie)); break; default: - a->Weird(fmt("gtp_invalid_info_element_%d", (*v)[i]->type())); + a->Weird("gtp_invalid_info_element", fmt("%d", (*v)[i]->type())); break; } } @@ -526,7 +526,7 @@ void UpdatePDP_Response(const BroAnalyzer& a, const GTPv1_Header* pdu) rv->Assign(9, BuildPrivateExt(ie)); break; default: - a->Weird(fmt("gtp_invalid_info_element_%d", (*v)[i]->type())); + a->Weird("gtp_invalid_info_element", fmt("%d", (*v)[i]->type())); break; } } @@ -560,7 +560,7 @@ void DeletePDP_Request(const BroAnalyzer& a, const GTPv1_Header* pdu) rv->Assign(2, BuildPrivateExt(ie)); break; default: - a->Weird(fmt("gtp_invalid_info_element_%d", (*v)[i]->type())); + a->Weird("gtp_invalid_info_element", fmt("%d", (*v)[i]->type())); break; } } @@ -591,7 +591,7 @@ void DeletePDP_Response(const BroAnalyzer& a, const GTPv1_Header* pdu) rv->Assign(1, BuildPrivateExt(ie)); break; default: - a->Weird(fmt("gtp_invalid_info_element_%d", (*v)[i]->type())); + a->Weird("gtp_invalid_info_element", fmt("%d", (*v)[i]->type())); break; } } diff --git a/src/analyzer/protocol/netbios/NetbiosSSN.cc b/src/analyzer/protocol/netbios/NetbiosSSN.cc index 07c81f6839..492375b7aa 100644 --- a/src/analyzer/protocol/netbios/NetbiosSSN.cc +++ b/src/analyzer/protocol/netbios/NetbiosSSN.cc @@ -97,7 +97,7 @@ int NetbiosSSN_Interpreter::ParseMessage(unsigned int type, unsigned int flags, return ParseDatagram(data, len, is_query); default: - analyzer->Weird(fmt("unknown_netbios_type: 0x%x", type)); + analyzer->Weird("unknown_netbios_type", fmt("0x%x", type)); return 1; } } @@ -143,7 +143,7 @@ int NetbiosSSN_Interpreter::ParseMessageTCP(const u_char* data, int len, NetbiosSSN_RawMsgHdr hdr(data, len); if ( hdr.length > unsigned(len) ) - analyzer->Weird(fmt("excess_netbios_hdr_len (%d > %d)", + analyzer->Weird("excess_netbios_hdr_len", fmt("(%d > %d)", hdr.length, len)); else if ( hdr.length < unsigned(len) ) @@ -162,12 +162,12 @@ int NetbiosSSN_Interpreter::ParseMessageUDP(const u_char* data, int len, NetbiosDGM_RawMsgHdr hdr(data, len); if ( unsigned(hdr.length-14) > unsigned(len) ) - analyzer->Weird(fmt("excess_netbios_hdr_len (%d > %d)", + analyzer->Weird("excess_netbios_hdr_len", fmt("(%d > %d)", hdr.length, len)); else if ( hdr.length < unsigned(len) ) { - analyzer->Weird(fmt("deficit_netbios_hdr_len (%d < %d)", + analyzer->Weird("deficit_netbios_hdr_len", fmt("(%d < %d)", hdr.length, len)); len = hdr.length; } diff --git a/src/analyzer/protocol/rpc/MOUNT.cc b/src/analyzer/protocol/rpc/MOUNT.cc index f32f4449af..604d2e3ed1 100644 --- a/src/analyzer/protocol/rpc/MOUNT.cc +++ b/src/analyzer/protocol/rpc/MOUNT.cc @@ -17,7 +17,7 @@ using namespace analyzer::rpc; int MOUNT_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) { if ( c->Program() != 100005 ) - Weird(fmt("bad_RPC_program (%d)", c->Program())); + Weird("bad_RPC_program", fmt("%d", c->Program())); uint32 proc = c->Proc(); // The call arguments, depends on the call type obviously ... @@ -49,7 +49,7 @@ int MOUNT_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) n = 0; } else - Weird(fmt("unknown_MOUNT_request(%u)", proc)); + Weird("unknown_MOUNT_request", fmt("%u", proc)); // Return 1 so that replies to unprocessed calls will still // be processed, and the return status extracted. diff --git a/src/analyzer/protocol/rpc/NFS.cc b/src/analyzer/protocol/rpc/NFS.cc index 6d0841900c..ff16812d65 100644 --- a/src/analyzer/protocol/rpc/NFS.cc +++ b/src/analyzer/protocol/rpc/NFS.cc @@ -17,7 +17,7 @@ using namespace analyzer::rpc; int NFS_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) { if ( c->Program() != 100003 ) - Weird(fmt("bad_RPC_program (%d)", c->Program())); + Weird("bad_RPC_program", fmt("%d", c->Program())); uint32 proc = c->Proc(); // The call arguments, depends on the call type obviously ... @@ -103,7 +103,7 @@ int NFS_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) n = 0; } else - Weird(fmt("unknown_NFS_request(%u)", proc)); + Weird("unknown_NFS_request", fmt("%u", proc)); // Return 1 so that replies to unprocessed calls will still // be processed, and the return status extracted. diff --git a/src/analyzer/protocol/rpc/RPC.cc b/src/analyzer/protocol/rpc/RPC.cc index 9d86210df6..5bd748d1ea 100644 --- a/src/analyzer/protocol/rpc/RPC.cc +++ b/src/analyzer/protocol/rpc/RPC.cc @@ -371,9 +371,9 @@ void RPC_Interpreter::Event_RPC_Reply(uint32_t xid, BifEnum::rpc_status status, } } -void RPC_Interpreter::Weird(const char* msg) +void RPC_Interpreter::Weird(const char* msg, const char* addl) { - analyzer->Weird(msg); + analyzer->Weird(msg, addl); } @@ -532,9 +532,7 @@ bool Contents_RPC::CheckResync(int& len, const u_char*& data, bool orig) DEBUG_MSG("%.6f RPC resync: " "discard small pieces: %d\n", network_time, len); - Conn()->Weird( - fmt("RPC resync: discard %d bytes\n", - len)); + Conn()->Weird("RPC_resync", fmt("discard %d bytes\n", len)); } NeedResync(); @@ -677,7 +675,7 @@ void Contents_RPC::DeliverStream(int len, const u_char* data, bool orig) // network_time, IsOrig(), marker, last_frag, msg_buf.GetExpected(), msg_buf.GetProcessed(), len); if ( ! msg_buf.AddToExpected(marker) ) - Conn()->Weird(fmt("RPC_message_too_long (%" PRId64 ")" , msg_buf.GetExpected())); + Conn()->Weird("RPC_message_too_long", fmt("%" PRId64, msg_buf.GetExpected())); if ( last_frag ) state = WAIT_FOR_LAST_DATA; diff --git a/src/analyzer/protocol/rpc/RPC.h b/src/analyzer/protocol/rpc/RPC.h index 8fa19b8d53..40c65a00d4 100644 --- a/src/analyzer/protocol/rpc/RPC.h +++ b/src/analyzer/protocol/rpc/RPC.h @@ -123,7 +123,7 @@ protected: void Event_RPC_Call(RPC_CallInfo* c); void Event_RPC_Reply(uint32_t xid, BifEnum::rpc_status status, int reply_len); - void Weird(const char* name); + void Weird(const char* name, const char* addl = ""); PDict(RPC_CallInfo) calls; analyzer::Analyzer* analyzer; diff --git a/src/analyzer/protocol/socks/socks-analyzer.pac b/src/analyzer/protocol/socks/socks-analyzer.pac index 206b632fe0..f625851d0a 100644 --- a/src/analyzer/protocol/socks/socks-analyzer.pac +++ b/src/analyzer/protocol/socks/socks-analyzer.pac @@ -161,13 +161,13 @@ refine connection SOCKS_Conn += { function socks5_unsupported_authentication_method(auth_method: uint8): bool %{ - reporter->Weird(bro_analyzer()->Conn(), fmt("socks5_unsupported_authentication_method_%d", auth_method)); + reporter->Weird(bro_analyzer()->Conn(), "socks5_unsupported_authentication_method", fmt("%d", auth_method)); return true; %} function socks5_unsupported_authentication_version(auth_method: uint8, version: uint8): bool %{ - reporter->Weird(bro_analyzer()->Conn(), fmt("socks5_unsupported_authentication_%d_%d", auth_method, version)); + reporter->Weird(bro_analyzer()->Conn(), "socks5_unsupported_authentication", fmt("method %d, version %d", auth_method, version)); return true; %} diff --git a/src/analyzer/protocol/ssh/ssh-protocol.pac b/src/analyzer/protocol/ssh/ssh-protocol.pac index bf09f6e168..b0caebc740 100644 --- a/src/analyzer/protocol/ssh/ssh-protocol.pac +++ b/src/analyzer/protocol/ssh/ssh-protocol.pac @@ -415,7 +415,7 @@ refine connection SSH_Conn += { return true; - bro_analyzer()->Weird(fmt("ssh_unknown_kex_algorithm=%s", c_str(kex_algorithm_))); + bro_analyzer()->Weird("ssh_unknown_kex_algorithm", c_str(kex_algorithm_)); return true; } diff --git a/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac b/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac index 7d2986efe3..5cf250c366 100644 --- a/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac +++ b/src/analyzer/protocol/ssl/tls-handshake-analyzer.pac @@ -172,7 +172,7 @@ refine connection Handshake_Conn += { ServerName* servername = (*list)[i]; if ( servername->name_type() != 0 ) { - bro_analyzer()->Weird(fmt("Encountered unknown type in server name ssl extension: %d", servername->name_type())); + bro_analyzer()->Weird("ssl_ext_unknown_server_name_type", fmt("%d", servername->name_type())); continue; } diff --git a/src/analyzer/protocol/udp/UDP.cc b/src/analyzer/protocol/udp/UDP.cc index ae56d8d22d..ca144941b6 100644 --- a/src/analyzer/protocol/udp/UDP.cc +++ b/src/analyzer/protocol/udp/UDP.cc @@ -124,7 +124,7 @@ void UDP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig, int ulen = ntohs(up->uh_ulen); if ( ulen != len ) - Weird(fmt("UDP_datagram_length_mismatch(%d!=%d)", ulen, len)); + Weird("UDP_datagram_length_mismatch", fmt("%d != %d", ulen, len)); len -= sizeof(struct udphdr); ulen -= sizeof(struct udphdr); diff --git a/src/event.bif b/src/event.bif index 28ed7f6807..ae00c9b653 100644 --- a/src/event.bif +++ b/src/event.bif @@ -421,7 +421,7 @@ event conn_stats%(c: connection, os: endpoint_stats, rs: endpoint_stats%); ## ## addl: Optional additional context further describing the situation. ## -## .. bro:see:: flow_weird net_weird +## .. bro:see:: flow_weird net_weird file_weird ## ## .. note:: "Weird" activity is much more common in real-world network traffic ## than one would intuitively expect. While in principle, any protocol @@ -444,7 +444,7 @@ event conn_weird%(name: string, c: connection, addl: string%); ## ## dst: The destination address corresponding to the activity. ## -## .. bro:see:: conn_weird net_weird +## .. bro:see:: conn_weird net_weird file_weird ## ## .. note:: "Weird" activity is much more common in real-world network traffic ## than one would intuitively expect. While in principle, any protocol @@ -462,7 +462,7 @@ event flow_weird%(name: string, src: addr, dst: addr%); ## scripts use this name in filtering policies that specify which ## "weirds" are worth reporting. ## -## .. bro:see:: flow_weird +## .. bro:see:: flow_weird file_weird ## ## .. note:: "Weird" activity is much more common in real-world network traffic ## than one would intuitively expect. While in principle, any protocol @@ -470,6 +470,27 @@ event flow_weird%(name: string, src: addr, dst: addr%); ## endpoint's implementation interprets an RFC quite liberally. event net_weird%(name: string%); +## Generated for unexpected activity that is tied to a file. +## When Bro's packet analysis encounters activity that +## does not conform to a protocol's specification, it raises one of the +## ``*_weird`` events to report that. +## +## name: A unique name for the specific type of "weird" situation. Bro's default +## scripts use this name in filtering policies that specify which +## "weirds" are worth reporting. +## +## f: The corresponding file. +## +## addl: Additional information related to the weird. +## +## .. bro:see:: flow_weird net_weird conn_weird +## +## .. note:: "Weird" activity is much more common in real-world network traffic +## than one would intuitively expect. While in principle, any protocol +## violation could be an attack attempt, it's much more likely that an +## endpoint's implementation interprets an RFC quite liberally. +event file_weird%(name: string, f: fa_file, addl: string%); + ## Generated regularly for the purpose of profiling Bro's processing. This event ## is raised for every :bro:id:`load_sample_freq` packet. For these packets, ## Bro records script-level functions executed during their processing as well diff --git a/src/file_analysis/File.cc b/src/file_analysis/File.cc index 92c4356bda..641943909e 100644 --- a/src/file_analysis/File.cc +++ b/src/file_analysis/File.cc @@ -649,3 +649,9 @@ void File::FileEvent(EventHandlerPtr h, val_list* vl) analyzers.DrainModifications(); } } + +bool File::PermitWeird(const char* name, uint64 threshold, uint64 rate, + double duration) + { + return ::PermitWeird(weird_state, name, threshold, rate, duration); + } diff --git a/src/file_analysis/File.h b/src/file_analysis/File.h index 1d4fb03789..0c4c313f06 100644 --- a/src/file_analysis/File.h +++ b/src/file_analysis/File.h @@ -13,6 +13,7 @@ #include "Tag.h" #include "AnalyzerSet.h" #include "BroString.h" +#include "WeirdState.h" namespace file_analysis { @@ -192,6 +193,13 @@ public: */ bool SetMime(const string& mime_type); + /** + * Whether to permit a weird to carry on through the full reporter/weird + * framework. + */ + bool PermitWeird(const char* name, uint64 threshold, uint64 rate, + double duration); + protected: friend class Manager; friend class FileReassembler; @@ -325,6 +333,8 @@ protected: BroString::CVec chunks; } bof_buffer; /**< Beginning of file buffer. */ + WeirdStateMap weird_state; + static int id_idx; static int parent_id_idx; static int source_idx; diff --git a/src/file_analysis/analyzer/x509/OCSP.cc b/src/file_analysis/analyzer/x509/OCSP.cc index b5ec4f30c6..c49481c23a 100644 --- a/src/file_analysis/analyzer/x509/OCSP.cc +++ b/src/file_analysis/analyzer/x509/OCSP.cc @@ -160,11 +160,11 @@ bool file_analysis::OCSP::EndOfFile() if (!req) { - reporter->Weird(fmt("OPENSSL Could not parse OCSP request (fuid %s)", GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "openssl_ocsp_request_parse_error"); return false; } - ParseRequest(req, GetFile()->GetID().c_str()); + ParseRequest(req); OCSP_REQUEST_free(req); } else @@ -173,12 +173,12 @@ bool file_analysis::OCSP::EndOfFile() if (!resp) { - reporter->Weird(fmt("OPENSSL Could not parse OCSP response (fuid %s)", GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "openssl_ocsp_response_parse_error"); return false; } OCSP_RESPVal* resp_val = new OCSP_RESPVal(resp); // resp_val takes ownership - ParseResponse(resp_val, GetFile()->GetID().c_str()); + ParseResponse(resp_val); Unref(resp_val); } @@ -412,7 +412,7 @@ static uint64 parse_request_version(OCSP_REQUEST* req) } #endif -void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req, const char* fid) +void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req) { char buf[OCSP_STRING_BUF_SIZE]; // we need a buffer for some of the openssl functions memset(buf, 0, sizeof(buf)); @@ -453,7 +453,7 @@ void file_analysis::OCSP::ParseRequest(OCSP_REQUEST* req, const char* fid) BIO_free(bio); } -void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) +void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val) { OCSP_RESPONSE *resp = resp_val->GetResp(); //OCSP_RESPBYTES *resp_bytes = resp->responseBytes; @@ -532,7 +532,7 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) produced_at = OCSP_resp_get0_produced_at(basic_resp); #endif - vl->append(new Val(GetTimeFromAsn1(produced_at, fid, reporter), TYPE_TIME)); + vl->append(new Val(GetTimeFromAsn1(produced_at, GetFile(), reporter), TYPE_TIME)); // responses @@ -579,7 +579,7 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) // revocation time and reason if revoked if ( status == V_OCSP_CERTSTATUS_REVOKED ) { - rvl->append(new Val(GetTimeFromAsn1(revoke_time, fid, reporter), TYPE_TIME)); + rvl->append(new Val(GetTimeFromAsn1(revoke_time, GetFile(), reporter), TYPE_TIME)); if ( reason != OCSP_REVOKED_STATUS_NOSTATUS ) { @@ -596,12 +596,12 @@ void file_analysis::OCSP::ParseResponse(OCSP_RESPVal *resp_val, const char* fid) } if ( this_update ) - rvl->append(new Val(GetTimeFromAsn1(this_update, fid, reporter), TYPE_TIME)); + rvl->append(new Val(GetTimeFromAsn1(this_update, GetFile(), reporter), TYPE_TIME)); else rvl->append(new Val(0.0, TYPE_TIME)); if ( next_update ) - rvl->append(new Val(GetTimeFromAsn1(next_update, fid, reporter), TYPE_TIME)); + rvl->append(new Val(GetTimeFromAsn1(next_update, GetFile(), reporter), TYPE_TIME)); else rvl->append(new Val(0.0, TYPE_TIME)); diff --git a/src/file_analysis/analyzer/x509/OCSP.h b/src/file_analysis/analyzer/x509/OCSP.h index 75caf3120a..eb6499794c 100644 --- a/src/file_analysis/analyzer/x509/OCSP.h +++ b/src/file_analysis/analyzer/x509/OCSP.h @@ -29,8 +29,8 @@ protected: OCSP(RecordVal* args, File* file, bool request); private: - void ParseResponse(OCSP_RESPVal*, const char* fid = 0); - void ParseRequest(OCSP_REQUEST*, const char* fid = 0); + void ParseResponse(OCSP_RESPVal*); + void ParseRequest(OCSP_REQUEST*); void ParseExtensionsSpecific(X509_EXTENSION* ex, bool, ASN1_OBJECT*, const char*) override; std::string ocsp_data; diff --git a/src/file_analysis/analyzer/x509/X509.cc b/src/file_analysis/analyzer/x509/X509.cc index e34bf58d82..38422897db 100644 --- a/src/file_analysis/analyzer/x509/X509.cc +++ b/src/file_analysis/analyzer/x509/X509.cc @@ -47,14 +47,14 @@ bool file_analysis::X509::EndOfFile() ::X509* ssl_cert = d2i_X509(NULL, &cert_char, cert_data.size()); if ( ! ssl_cert ) { - reporter->Weird(fmt("Could not parse X509 certificate (fuid %s)", GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "x509_cert_parse_error"); return false; } X509Val* cert_val = new X509Val(ssl_cert); // cert_val takes ownership of ssl_cert // parse basic information into record. - RecordVal* cert_record = ParseCertificate(cert_val, GetFile()->GetID().c_str()); + RecordVal* cert_record = ParseCertificate(cert_val, GetFile()); // and send the record on to scriptland val_list* vl = new val_list(); @@ -86,7 +86,7 @@ bool file_analysis::X509::EndOfFile() return false; } -RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val, const char* fid) +RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val, File* f) { ::X509* ssl_cert = cert_val->GetCertificate(); @@ -133,8 +133,8 @@ RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val, const char* pX509Cert->Assign(3, new StringVal(len, buf)); BIO_free(bio); - pX509Cert->Assign(5, new Val(GetTimeFromAsn1(X509_get_notBefore(ssl_cert), fid, reporter), TYPE_TIME)); - pX509Cert->Assign(6, new Val(GetTimeFromAsn1(X509_get_notAfter(ssl_cert), fid, reporter), TYPE_TIME)); + pX509Cert->Assign(5, new Val(GetTimeFromAsn1(X509_get_notBefore(ssl_cert), f, reporter), TYPE_TIME)); + pX509Cert->Assign(6, new Val(GetTimeFromAsn1(X509_get_notAfter(ssl_cert), f, reporter), TYPE_TIME)); // we only read 255 bytes because byte 256 is always 0. // if the string is longer than 255, that will be our null-termination, @@ -236,7 +236,7 @@ void file_analysis::X509::ParseBasicConstraints(X509_EXTENSION* ex) } else - reporter->Weird(fmt("Certificate with invalid BasicConstraint. fuid %s", GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "x509_invalid_basic_constraint"); } void file_analysis::X509::ParseExtensionsSpecific(X509_EXTENSION* ex, bool global, ASN1_OBJECT* ext_asn, const char* oid) @@ -266,7 +266,7 @@ void file_analysis::X509::ParseSAN(X509_EXTENSION* ext) GENERAL_NAMES *altname = (GENERAL_NAMES*)X509V3_EXT_d2i(ext); if ( ! altname ) { - reporter->Weird(fmt("Could not parse subject alternative names. fuid %s", GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "x509_san_parse_error"); return; } @@ -286,7 +286,7 @@ void file_analysis::X509::ParseSAN(X509_EXTENSION* ext) { if ( ASN1_STRING_type(gen->d.ia5) != V_ASN1_IA5STRING ) { - reporter->Weird(fmt("DNS-field does not contain an IA5String. fuid %s", GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "x509_san_non_string"); continue; } @@ -337,7 +337,7 @@ void file_analysis::X509::ParseSAN(X509_EXTENSION* ext) else { - reporter->Weird(fmt("Weird IP address length %d in subject alternative name. fuid %s", gen->d.ip->length, GetFile()->GetID().c_str())); + reporter->Weird(GetFile(), "x509_san_ip_length", fmt("%d", gen->d.ip->length)); continue; } } diff --git a/src/file_analysis/analyzer/x509/X509.h b/src/file_analysis/analyzer/x509/X509.h index 91a5a7a5a1..a3dc62e533 100644 --- a/src/file_analysis/analyzer/x509/X509.h +++ b/src/file_analysis/analyzer/x509/X509.h @@ -79,13 +79,13 @@ public: * * @param cert_val The certificate to converts. * - * @param fid A file ID associated with the certificate, if any + * @param f A file associated with the certificate, if any * (primarily for error reporting). * * @param Returns the new record value and passes ownership to * caller. */ - static RecordVal* ParseCertificate(X509Val* cert_val, const char* fid = 0); + static RecordVal* ParseCertificate(X509Val* cert_val, File* file = 0); static file_analysis::Analyzer* Instantiate(RecordVal* args, File* file) { return new X509(args, file); } diff --git a/src/file_analysis/analyzer/x509/X509Common.cc b/src/file_analysis/analyzer/x509/X509Common.cc index d59a383b78..b6c16fc1dc 100644 --- a/src/file_analysis/analyzer/x509/X509Common.cc +++ b/src/file_analysis/analyzer/x509/X509Common.cc @@ -20,9 +20,16 @@ X509Common::X509Common(file_analysis::Tag arg_tag, RecordVal* arg_args, File* ar { } -double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, Reporter* reporter) +static void EmitWeird(const char* name, File* file, const char* addl = "") + { + if ( file ) + reporter->Weird(file, name, addl); + else + reporter->Weird(name); + } + +double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, File* f, Reporter* reporter) { - const char *fid = arg_fid ? arg_fid : ""; time_t lResult = 0; char lBuffer[26]; @@ -35,14 +42,14 @@ double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, { if ( remaining < 11 || remaining > 17 ) { - reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- UTCTime has wrong length", fid)); + EmitWeird("x509_utc_length", f); return 0; } if ( pString[remaining-1] != 'Z' ) { // not valid according to RFC 2459 4.1.2.5.1 - reporter->Weird(fmt("Could not parse UTC time in non-YY-format in X509 certificate (x509 %s)", fid)); + EmitWeird("x509_utc_format", f); return 0; } @@ -71,7 +78,7 @@ double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, if ( remaining < 12 || remaining > 23 ) { - reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- Generalized time has wrong length", fid)); + EmitWeird("x509_gen_time_length", f); return 0; } @@ -82,7 +89,7 @@ double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, } else { - reporter->Weird(fmt("Invalid time type in X509 certificate (fuid %s)", fid)); + EmitWeird("x509_invalid_time_type", f); return 0; } @@ -115,7 +122,7 @@ double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, else { - reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- additional char after time", fid)); + EmitWeird("x509_time_add_char", f); return 0; } @@ -130,13 +137,13 @@ double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, { if ( remaining < 5 ) { - reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- not enough bytes remaining for offset", fid)); + EmitWeird("x509_time_offset_underflow", f); return 0; } if ((*pString != '+') && (*pString != '-')) { - reporter->Weird(fmt("Could not parse time in X509 certificate (fuid %s) -- unknown offset type", fid)); + EmitWeird("x509_time_offset_type", f); return 0; } @@ -249,7 +256,7 @@ void file_analysis::X509Common::ParseExtension(X509_EXTENSION* ex, EventHandlerP } } - StringVal* ext_val = GetExtensionFromBIO(bio); + StringVal* ext_val = GetExtensionFromBIO(bio, GetFile()); if ( ! ext_val ) ext_val = new StringVal(0, ""); @@ -282,7 +289,7 @@ void file_analysis::X509Common::ParseExtension(X509_EXTENSION* ex, EventHandlerP ParseExtensionsSpecific(ex, global, ext_asn, oid); } -StringVal* file_analysis::X509Common::GetExtensionFromBIO(BIO* bio) +StringVal* file_analysis::X509Common::GetExtensionFromBIO(BIO* bio, File* f) { BIO_flush(bio); ERR_clear_error(); @@ -292,7 +299,7 @@ StringVal* file_analysis::X509Common::GetExtensionFromBIO(BIO* bio) { char tmp[120]; ERR_error_string_n(ERR_get_error(), tmp, sizeof(tmp)); - reporter->Weird(fmt("X509::GetExtensionFromBIO: %s", tmp)); + EmitWeird("x509_get_ext_from_bio", f, tmp); BIO_free_all(bio); return 0; } diff --git a/src/file_analysis/analyzer/x509/X509Common.h b/src/file_analysis/analyzer/x509/X509Common.h index a7015bc235..2f02357cca 100644 --- a/src/file_analysis/analyzer/x509/X509Common.h +++ b/src/file_analysis/analyzer/x509/X509Common.h @@ -25,11 +25,13 @@ public: * @param bio the OpenSSL BIO to read. It will be freed by the function, * including when an error occurs. * + * @param f an associated file, if any (used for error reporting). + * * @return The X509 extension value. */ - static StringVal* GetExtensionFromBIO(BIO* bio); + static StringVal* GetExtensionFromBIO(BIO* bio, File* f = 0); - static double GetTimeFromAsn1(const ASN1_TIME* atime, const char* arg_fid, Reporter* reporter); + static double GetTimeFromAsn1(const ASN1_TIME* atime, File* f, Reporter* reporter); protected: X509Common(file_analysis::Tag arg_tag, RecordVal* arg_args, File* arg_file); diff --git a/src/reporter.bif b/src/reporter.bif index 4a58e2728b..71fc50b49d 100644 --- a/src/reporter.bif +++ b/src/reporter.bif @@ -127,6 +127,27 @@ function Reporter::conn_weird%(name: string, c: connection, addl: string &defaul return val_mgr->GetBool(1); %} +## Generates a "file" weird. +## +## name: the name of the weird. +## +## f: the file associated with the weird. +## +## addl: additional information to accompany the weird. +## +## Returns: true if the file was still valid, else false. +function Reporter::file_weird%(name: string, f: fa_file, addl: string &default=""%): bool + %{ + auto fuid = f->AsRecordVal()->Lookup(0)->AsStringVal(); + auto file = file_mgr->LookupFile(fuid->CheckString()); + + if ( ! file ) + return val_mgr->GetBool(0); + + reporter->Weird(file, name->CheckString(), addl->CheckString()); + return val_mgr->GetBool(1); + %} + ## Gets the weird sampling whitelist ## ## Returns: Current weird sampling whitelist diff --git a/testing/btest/Baseline/core.disable-mobile-ipv6/weird.log b/testing/btest/Baseline/core.disable-mobile-ipv6/weird.log index 9da1a8d3ba..ee45663170 100644 --- a/testing/btest/Baseline/core.disable-mobile-ipv6/weird.log +++ b/testing/btest/Baseline/core.disable-mobile-ipv6/weird.log @@ -6,5 +6,5 @@ #open 2012-04-05-21-56-51 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #types time string addr port addr port string string bool string -1333663011.602839 - - - - - unknown_protocol_135 - F bro +1333663011.602839 - - - - - unknown_protocol - F bro #close 2012-04-05-21-56-51 diff --git a/testing/btest/Baseline/scripts.base.protocols.modbus.events/conn.log b/testing/btest/Baseline/scripts.base.protocols.modbus.events/conn.log index dbdf6b2bef..964ae1dede 100644 --- a/testing/btest/Baseline/scripts.base.protocols.modbus.events/conn.log +++ b/testing/btest/Baseline/scripts.base.protocols.modbus.events/conn.log @@ -3,7 +3,7 @@ #empty_field (empty) #unset_field - #path conn -#open 2016-07-13-16-16-30 +#open 2019-04-02-01-01-40 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents #types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] 1093521678.945447 CHhAvVGS1DHFjwGM9 10.0.0.57 2387 10.0.0.3 502 tcp - 0.000493 0 0 SF - - 0 FafA 2 80 2 80 - @@ -13,6 +13,6 @@ 1093522946.554059 CUM0KZ3MLUfNB0cl11 10.0.0.57 2585 10.0.0.8 502 tcp - 76.561880 926 0 SF - - 0 ShADafF 8 1254 7 288 - 1093523065.562221 CmES5u32sYpV7JYN 10.0.0.8 502 10.0.0.57 4446 tcp - 155.114237 128 0 SF - - 0 ShADaFf 16 776 15 608 - 1153491879.610371 CP5puj4I8PtEU4qzYg 192.168.66.235 2582 166.161.16.230 502 tcp - 2.905078 0 0 S0 - - 0 S 2 96 0 0 - -1153491888.530306 C37jN32gN3y3AZzyf6 192.168.66.235 2582 166.161.16.230 502 tcp modbus 85.560847 1692 1278 S1 - - 0 ShADad 167 8380 181 8522 - +1153491888.530306 C37jN32gN3y3AZzyf6 192.168.66.235 2582 166.161.16.230 502 tcp - 85.560847 1692 1278 S1 - - 0 ShADad 167 8380 181 8522 - 1342774499.588269 C3eiCBGOLw3VtHfOj 10.1.1.234 51411 10.10.5.85 502 tcp modbus 2100.811351 237936 4121200 S2 - - 0 ShADdaF 39659 2300216 20100 5166412 - -#close 2016-07-13-16-16-33 +#close 2019-04-02-01-01-42 diff --git a/testing/btest/Baseline/scripts.base.protocols.modbus.exception_handling/weird.log b/testing/btest/Baseline/scripts.base.protocols.modbus.exception_handling/weird.log deleted file mode 100644 index 00c668fb27..0000000000 --- a/testing/btest/Baseline/scripts.base.protocols.modbus.exception_handling/weird.log +++ /dev/null @@ -1,13 +0,0 @@ -#separator \x09 -#set_separator , -#empty_field (empty) -#unset_field - -#path weird -#open 2016-07-13-16-16-39 -#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer -#types time string addr port addr port string string bool string -1153491909.414066 - - - - - truncated_IP - F bro -1153491912.529443 CHhAvVGS1DHFjwGM9 192.168.66.235 2582 166.161.16.230 502 binpac exception: out_of_bound: WriteSingleRegisterRequest: 4 > 0 - F bro -1153491920.661039 CHhAvVGS1DHFjwGM9 192.168.66.235 2582 166.161.16.230 502 TCP_ack_underflow_or_misorder - F bro -1153491929.715910 CHhAvVGS1DHFjwGM9 192.168.66.235 2582 166.161.16.230 502 TCP_seq_underflow_or_misorder - F bro -#close 2016-07-13-16-16-39 diff --git a/testing/btest/Baseline/scripts.base.protocols.modbus.length_mismatch/weird.log b/testing/btest/Baseline/scripts.base.protocols.modbus.length_mismatch/weird.log deleted file mode 100644 index 800b9ec4eb..0000000000 --- a/testing/btest/Baseline/scripts.base.protocols.modbus.length_mismatch/weird.log +++ /dev/null @@ -1,10 +0,0 @@ -#separator \x09 -#set_separator , -#empty_field (empty) -#unset_field - -#path weird -#open 2018-08-30-14-12-39 -#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer -#types time string addr port addr port string string bool string -1445502056.228889 CHhAvVGS1DHFjwGM9 192.168.2.166 1987 192.168.88.95 502 binpac exception: out_of_bound: ReadWriteMultipleRegistersRequest:write_register_values: 16932 > 191 - F bro -#close 2018-08-30-14-12-39 diff --git a/testing/btest/scripts/base/protocols/modbus/events.bro b/testing/btest/scripts/base/protocols/modbus/events.bro index fe748fa3dc..55a3f3cb04 100644 --- a/testing/btest/scripts/base/protocols/modbus/events.bro +++ b/testing/btest/scripts/base/protocols/modbus/events.bro @@ -7,6 +7,7 @@ # @TEST-EXEC: btest-diff coverage # @TEST-EXEC: btest-diff conn.log +redef DPD::ignore_violations_after = 1; event modbus_message(c: connection, headers: ModbusHeaders, is_orig: bool) { diff --git a/testing/btest/scripts/base/protocols/modbus/exception_handling.test b/testing/btest/scripts/base/protocols/modbus/exception_handling.test index 9d15c754f9..8a4fadcbeb 100644 --- a/testing/btest/scripts/base/protocols/modbus/exception_handling.test +++ b/testing/btest/scripts/base/protocols/modbus/exception_handling.test @@ -1,10 +1,8 @@ # @TEST-EXEC: bro -r $TRACES/modbus/fuzz-72.trace # @TEST-EXEC: btest-diff modbus.log -# @TEST-EXEC: btest-diff weird.log # The pcap has a flow with some fuzzed modbus traffic in it that should cause # the binpac-generated analyzer code to throw a binpac::ExceptionOutOfBound. # This should be correctly caught as a type of binpac::Exception and the # binpac::ModbusTCP::Exception type that's defined as part of the analyzer # shouldn't interfere with that handling and definitely shouldn't crash bro. -# A weird is currently emitted for parsing exceptions. diff --git a/testing/btest/scripts/base/protocols/modbus/length_mismatch.bro b/testing/btest/scripts/base/protocols/modbus/length_mismatch.bro index 35835e4b64..17371f3788 100644 --- a/testing/btest/scripts/base/protocols/modbus/length_mismatch.bro +++ b/testing/btest/scripts/base/protocols/modbus/length_mismatch.bro @@ -12,4 +12,3 @@ # data buffer. # @TEST-EXEC: bro -r $TRACES/modbus/4SICS-GeekLounge-151022-min.pcap -# @TEST-EXEC: btest-diff weird.log diff --git a/testing/btest/scripts/base/protocols/modbus/policy.bro b/testing/btest/scripts/base/protocols/modbus/policy.bro index b28ebd3b4b..8f7e41c274 100644 --- a/testing/btest/scripts/base/protocols/modbus/policy.bro +++ b/testing/btest/scripts/base/protocols/modbus/policy.bro @@ -7,3 +7,5 @@ @load protocols/modbus/known-masters-slaves.bro @load protocols/modbus/track-memmap.bro + +redef DPD::ignore_violations_after = 1; diff --git a/testing/external/commit-hash.zeek-testing b/testing/external/commit-hash.zeek-testing index 0a59fc487b..e202437bb2 100644 --- a/testing/external/commit-hash.zeek-testing +++ b/testing/external/commit-hash.zeek-testing @@ -1 +1 @@ -1de9cb3f7386e8243431f57b00d87b0ecf98e5ef +b9e28fecbc04c1fe37906f6d6078fb11114b6661 diff --git a/testing/external/commit-hash.zeek-testing-private b/testing/external/commit-hash.zeek-testing-private index c5e623029b..8366c51105 100644 --- a/testing/external/commit-hash.zeek-testing-private +++ b/testing/external/commit-hash.zeek-testing-private @@ -1 +1 @@ -47df32597eb14183ca0a468be4ab1525417d79dc +68e6f55eaaced3f4f42b8e291f6e97dd709833c0