From be7110f6c09dba2c255cd28cf1454d38cc8ee02f Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 14 Mar 2019 18:47:32 -0700 Subject: [PATCH 01/13] Make Syslog analyzer accept messages that omit Priority Essentially, it will now process/parse priority values if they are there, or else just accept whatever remaining data/text is there as the syslog message. Reasoning is that there's syslog producers out there that may have simply forgotten/neglected to send the priority value and technically won't conform to what the standard says, though we can infer the intent (some syslog consumers already may do similarly, but I didn't verify). --- doc | 2 +- scripts/base/protocols/syslog/consts.bro | 4 ++- .../protocol/syslog/syslog-analyzer.pac | 25 +++++++++++++----- .../protocol/syslog/syslog-protocol.pac | 23 +++++++++++++--- .../syslog.log | 10 +++++++ testing/btest/Traces/syslog-missing-pri.trace | Bin 0 -> 143 bytes .../base/protocols/syslog/missing-pri.bro | 4 +++ 7 files changed, 55 insertions(+), 13 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.syslog.missing-pri/syslog.log create mode 100755 testing/btest/Traces/syslog-missing-pri.trace create mode 100644 testing/btest/scripts/base/protocols/syslog/missing-pri.bro diff --git a/doc b/doc index 5849f875ea..11db899c89 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 5849f875ea6cae038d4881eba326256202e711be +Subproject commit 11db899c89686d551b539c069b4d2aec2ffd49c9 diff --git a/scripts/base/protocols/syslog/consts.bro b/scripts/base/protocols/syslog/consts.bro index dce1877ecf..c68cbda658 100644 --- a/scripts/base/protocols/syslog/consts.bro +++ b/scripts/base/protocols/syslog/consts.bro @@ -29,6 +29,7 @@ export { [21] = "LOCAL5", [22] = "LOCAL6", [23] = "LOCAL7", + [999] = "UNSPECIFIED", } &default=function(c: count): string { return fmt("?-%d", c); }; ## Mapping between the constants and string values for syslog severities. @@ -41,5 +42,6 @@ export { [5] = "NOTICE", [6] = "INFO", [7] = "DEBUG", + [999] = "UNSPECIFIED", } &default=function(c: count): string { return fmt("?-%d", c); }; -} \ No newline at end of file +} diff --git a/src/analyzer/protocol/syslog/syslog-analyzer.pac b/src/analyzer/protocol/syslog/syslog-analyzer.pac index 6657a63699..46e2cc171d 100644 --- a/src/analyzer/protocol/syslog/syslog-analyzer.pac +++ b/src/analyzer/protocol/syslog/syslog-analyzer.pac @@ -7,16 +7,27 @@ connection Syslog_Conn(bro_analyzer: BroAnalyzer) flow Syslog_Flow { - datagram = Syslog_Message withcontext(connection, this); + datagram = Syslog_Message_Optional_PRI withcontext(connection, this); function process_syslog_message(m: Syslog_Message): bool %{ - BifEvent::generate_syslog_message(connection()->bro_analyzer(), - connection()->bro_analyzer()->Conn(), - ${m.PRI.facility}, - ${m.PRI.severity}, - new StringVal(${m.msg}.length(), (const char*) ${m.msg}.begin()) - ); + if ( ${m.has_pri} ) + BifEvent::generate_syslog_message( + connection()->bro_analyzer(), + connection()->bro_analyzer()->Conn(), + ${m.PRI.facility}, + ${m.PRI.severity}, + new StringVal(${m.msg}.length(), (const char*)${m.msg}.begin()) + ); + else + BifEvent::generate_syslog_message( + connection()->bro_analyzer(), + connection()->bro_analyzer()->Conn(), + 999, + 999, + new StringVal(${m.msg}.length(), (const char*)${m.msg}.begin()) + ); + return true; %} diff --git a/src/analyzer/protocol/syslog/syslog-protocol.pac b/src/analyzer/protocol/syslog/syslog-protocol.pac index c1502fc534..41c42eba59 100644 --- a/src/analyzer/protocol/syslog/syslog-protocol.pac +++ b/src/analyzer/protocol/syslog/syslog-protocol.pac @@ -1,12 +1,27 @@ -type Syslog_Message = record { - PRI: Syslog_Priority; +type Syslog_Message_Optional_PRI = record { + lt: uint8; + after_lt: bytestring &restofdata &transient; +} +&byteorder = littleendian +&exportsourcedata +&let { + standard: Syslog_Message(true) withinput sourcedata &if(lt == 60); # '<' + nonstandard: Syslog_Message(false) withinput sourcedata &if(lt != 60); +}; + +type Syslog_Message(has_pri: bool) = record { + opt_pri: case has_pri of { + true -> PRI: Syslog_Priority; + false -> nothing: empty; + }; + msg: bytestring &restofdata; } &byteorder = littleendian; type Syslog_Priority = record { - lt : uint8; # &check(lt == 60); # '<' + lt : uint8 &enforce(lt == 60); # '<' val : RE/[[:digit:]]+/; - gt : uint8; # &check(gt == 62); # '>' + gt : uint8 &enforce(gt == 62); # '>' } &let { val_length: int = sizeof(val) - 1; int_val: int = bytestring_to_int(val, 10); diff --git a/testing/btest/Baseline/scripts.base.protocols.syslog.missing-pri/syslog.log b/testing/btest/Baseline/scripts.base.protocols.syslog.missing-pri/syslog.log new file mode 100644 index 0000000000..2a1faf440e --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.syslog.missing-pri/syslog.log @@ -0,0 +1,10 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path syslog +#open 2019-03-15-01-41-39 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto facility severity message +#types time string addr port addr port enum string string string +1552584410.781231 CHhAvVGS1DHFjwGM9 192.168.2.118 60786 192.168.2.21 514 udp UNSPECIFIED UNSPECIFIED This is not really a syslog message #173538 1552584410.781186 +#close 2019-03-15-01-41-39 diff --git a/testing/btest/Traces/syslog-missing-pri.trace b/testing/btest/Traces/syslog-missing-pri.trace new file mode 100755 index 0000000000000000000000000000000000000000..625ecddf749c9e3966d3e6f1ac8012e008965bb5 GIT binary patch literal 143 zcmca|c+)~A1{MYcU}0bca&Gl?#jJnL&5#acgYcx~4+AU`K1j@1=qSO#;L5-d`7NG- z!9nm}#DNt|Wk4+Ywup&|!PUzyBqOs}0f_SQOB9My6LWGZ6%rMSD~ogT(-m@4i;EM} fQx%jA&5ccsEffq*O^r+~OiT<7^vo>`4K2(79x5n@ literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/syslog/missing-pri.bro b/testing/btest/scripts/base/protocols/syslog/missing-pri.bro new file mode 100644 index 0000000000..c33eb1638b --- /dev/null +++ b/testing/btest/scripts/base/protocols/syslog/missing-pri.bro @@ -0,0 +1,4 @@ +# @TEST-EXEC: bro -r $TRACES/syslog-missing-pri.trace %INPUT +# @TEST-EXEC: btest-diff syslog.log + +@load base/protocols/syslog From 995368e68c3c882c67ca50d3a757c8b7d0d2f42f Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Mon, 1 Apr 2019 18:27:53 -0700 Subject: [PATCH 02/13] Remove variable content from weird names This changes many weird names to move non-static content from the weird name into the "addl" field to help ensure the total number of weird names is reasonably bounded. Note the net_weird and flow_weird events do not have an "addl" parameter, so information may no longer be available in those cases -- to make it available again we'd need to either (1) define new events that contain such a parameter, or (2) change net_weird/flow_weird event signature (which is a breaking change for user-code at the moment). Also, the generic handling of binpac exceptions for analyzers which to not otherwise catch and handle them has been changed from a Weird to a ProtocolViolation. Finally, a new "file_weird" event has been added for reporting weirdness found during file analysis. --- NEWS | 46 +++++++++++++++++++ doc | 2 +- scripts/base/frameworks/notice/weird.bro | 10 ++++ src/CMakeLists.txt | 1 + src/Conn.cc | 24 +--------- src/Conn.h | 10 +--- src/IP.cc | 4 +- src/Reporter.cc | 23 ++++++++-- src/Reporter.h | 4 +- src/Sessions.cc | 8 ++-- src/WeirdState.cc | 30 ++++++++++++ src/WeirdState.h | 21 +++++++++ src/analyzer/Analyzer.cc | 6 +-- .../protocol/bittorrent/BitTorrent.cc | 2 - .../protocol/bittorrent/BitTorrentTracker.cc | 9 ++-- .../bittorrent/bittorrent-analyzer.pac | 1 - .../protocol/dce-rpc/dce_rpc-auth.pac | 2 +- .../protocol/gtpv1/gtpv1-analyzer.pac | 12 ++--- src/analyzer/protocol/netbios/NetbiosSSN.cc | 8 ++-- src/analyzer/protocol/rpc/MOUNT.cc | 4 +- src/analyzer/protocol/rpc/NFS.cc | 4 +- src/analyzer/protocol/rpc/RPC.cc | 10 ++-- src/analyzer/protocol/rpc/RPC.h | 2 +- .../protocol/socks/socks-analyzer.pac | 4 +- src/analyzer/protocol/ssh/ssh-protocol.pac | 2 +- .../protocol/ssl/tls-handshake-analyzer.pac | 2 +- src/analyzer/protocol/udp/UDP.cc | 2 +- src/event.bif | 27 +++++++++-- src/file_analysis/File.cc | 6 +++ src/file_analysis/File.h | 10 ++++ src/file_analysis/analyzer/x509/OCSP.cc | 20 ++++---- src/file_analysis/analyzer/x509/OCSP.h | 4 +- src/file_analysis/analyzer/x509/X509.cc | 18 ++++---- src/file_analysis/analyzer/x509/X509.h | 4 +- src/file_analysis/analyzer/x509/X509Common.cc | 31 ++++++++----- src/file_analysis/analyzer/x509/X509Common.h | 6 ++- src/reporter.bif | 21 +++++++++ .../core.disable-mobile-ipv6/weird.log | 2 +- .../conn.log | 6 +-- .../weird.log | 13 ------ .../weird.log | 10 ---- .../scripts/base/protocols/modbus/events.bro | 1 + .../protocols/modbus/exception_handling.test | 2 - .../base/protocols/modbus/length_mismatch.bro | 1 - .../scripts/base/protocols/modbus/policy.bro | 2 + testing/external/commit-hash.zeek-testing | 2 +- .../external/commit-hash.zeek-testing-private | 2 +- 47 files changed, 289 insertions(+), 152 deletions(-) create mode 100644 src/WeirdState.cc create mode 100644 src/WeirdState.h delete mode 100644 testing/btest/Baseline/scripts.base.protocols.modbus.exception_handling/weird.log delete mode 100644 testing/btest/Baseline/scripts.base.protocols.modbus.length_mismatch/weird.log 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 From faa0f43558ce4d0b80bcc878fa9cfbd853299c3b Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 2 Apr 2019 10:41:32 -0700 Subject: [PATCH 03/13] Fix reporter net_weird API usage for unknown_mobility_type (This code path is pre-processed out by default) --- src/IP.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IP.cc b/src/IP.cc index 516be7fdd4..589c973e72 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", mob->ip6mob_type)); + reporter->Weird("unknown_mobility_type"); break; } From 3f7bbf2784d094787e6c7cb32adb0fc658fb8a86 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 3 Apr 2019 13:26:51 -0700 Subject: [PATCH 04/13] Update external test commit pointers --- testing/external/commit-hash.zeek-testing | 2 +- testing/external/commit-hash.zeek-testing-private | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/external/commit-hash.zeek-testing b/testing/external/commit-hash.zeek-testing index 0a59fc487b..029d39391b 100644 --- a/testing/external/commit-hash.zeek-testing +++ b/testing/external/commit-hash.zeek-testing @@ -1 +1 @@ -1de9cb3f7386e8243431f57b00d87b0ecf98e5ef +37f541404be417d5b092b8b36c7c1c84d2f307e9 diff --git a/testing/external/commit-hash.zeek-testing-private b/testing/external/commit-hash.zeek-testing-private index c5e623029b..a99b5e8d7b 100644 --- a/testing/external/commit-hash.zeek-testing-private +++ b/testing/external/commit-hash.zeek-testing-private @@ -1 +1 @@ -47df32597eb14183ca0a468be4ab1525417d79dc +de8e378210cacc599d8e59e1204286f7fe9cbc1b From 7c48aad5826738502765b2f079782ec2549d7b1c Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 4 Apr 2019 12:27:42 -0700 Subject: [PATCH 05/13] Update DTLS error handling DTLS now only outputs protocol violations once it saw something that looked like a DTLS connection (at least a client hello). Before the danger that it misinterprets something is too high. It has a configurable number of invalid packets that it can skip over (because other protocols might be interleaved with the connection) and a maximum amount of Protocol violations that it outputs because of wrong packet versions. --- doc | 2 +- scripts/base/init-bare.bro | 11 ++++++++ src/analyzer/protocol/ssl/CMakeLists.txt | 3 ++- src/analyzer/protocol/ssl/consts.bif | 2 ++ src/analyzer/protocol/ssl/dtls-protocol.pac | 27 ++++++++++++++++++- src/analyzer/protocol/ssl/dtls.pac | 1 + .../canonified_loaded_scripts.log | 5 ++-- .../canonified_loaded_scripts.log | 5 ++-- testing/btest/Baseline/plugins.hooks/output | 17 +++++++----- .../.stdout | 0 .../base/protocols/ssl/dtls-no-dtls.test | 15 +++++++++++ 11 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 src/analyzer/protocol/ssl/consts.bif create mode 100644 testing/btest/Baseline/scripts.base.protocols.ssl.dtls-no-dtls/.stdout create mode 100644 testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test diff --git a/doc b/doc index 5aa921f0f6..2036846610 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 5aa921f0f6ce2931e446a11f2a10cffb7f0dbc09 +Subproject commit 203684661040089877830eb98e12a6d4c18a4675 diff --git a/scripts/base/init-bare.bro b/scripts/base/init-bare.bro index 0c32cebcc5..e94efd07df 100644 --- a/scripts/base/init-bare.bro +++ b/scripts/base/init-bare.bro @@ -4169,6 +4169,17 @@ export { HashAlgorithm: count; ##< Hash algorithm number SignatureAlgorithm: count; ##< Signature algorithm number }; + + +## Number of non-DTLS frames that can occur in a DTLS connection before +## parsing of the connection is suspended. +## DTLS does not immediately stop parsing a connection because other protocols +## might be interleaved in the same UDP "connection". +const SSL::dtls_max_version_errors = 10 &redef; + +## Maximum number of invalid version errors to report in one DTLS connection. +const SSL::dtls_max_reported_version_errors = 1 &redef; + } module GLOBAL; diff --git a/src/analyzer/protocol/ssl/CMakeLists.txt b/src/analyzer/protocol/ssl/CMakeLists.txt index 14e41892c8..3193470635 100644 --- a/src/analyzer/protocol/ssl/CMakeLists.txt +++ b/src/analyzer/protocol/ssl/CMakeLists.txt @@ -8,6 +8,7 @@ bro_plugin_cc(SSL.cc DTLS.cc Plugin.cc) bro_plugin_bif(types.bif) bro_plugin_bif(events.bif) bro_plugin_bif(functions.bif) +bro_plugin_bif(consts.bif) bro_plugin_pac(tls-handshake.pac tls-handshake-protocol.pac tls-handshake-analyzer.pac ssl-defs.pac proc-client-hello.pac proc-server-hello.pac @@ -16,7 +17,7 @@ bro_plugin_pac(tls-handshake.pac tls-handshake-protocol.pac tls-handshake-analyz ) bro_plugin_pac(ssl.pac ssl-dtls-analyzer.pac ssl-analyzer.pac ssl-dtls-protocol.pac ssl-protocol.pac ssl-defs.pac proc-client-hello.pac - proc-server-hello.pac + proc-server-hello.pac proc-certificate.pac ) bro_plugin_pac(dtls.pac ssl-dtls-analyzer.pac dtls-analyzer.pac ssl-dtls-protocol.pac dtls-protocol.pac ssl-defs.pac) diff --git a/src/analyzer/protocol/ssl/consts.bif b/src/analyzer/protocol/ssl/consts.bif new file mode 100644 index 0000000000..9dcbaa65d5 --- /dev/null +++ b/src/analyzer/protocol/ssl/consts.bif @@ -0,0 +1,2 @@ +const SSL::dtls_max_version_errors: count; +const SSL::dtls_max_reported_version_errors: count; diff --git a/src/analyzer/protocol/ssl/dtls-protocol.pac b/src/analyzer/protocol/ssl/dtls-protocol.pac index 771aa267b3..70897a585c 100644 --- a/src/analyzer/protocol/ssl/dtls-protocol.pac +++ b/src/analyzer/protocol/ssl/dtls-protocol.pac @@ -45,15 +45,40 @@ type Handshake(rec: SSLRecord) = record { refine connection SSL_Conn += { + %member{ + uint16 invalid_version_count_; + uint16 reported_errors_; + %} + + %init{ + invalid_version_count_ = 0; + reported_errors_ = 0; + %} + function dtls_version_ok(version: uint16): uint16 %{ switch ( version ) { case DTLSv10: case DTLSv12: + // Reset only to 0 once we have seen a client hello. + // This means the connection gets a limited amount of valid/invalid + // packets before a client hello has to be seen - which seems reasonable. + if ( bro_analyzer()->ProtocolConfirmed() ) + invalid_version_count_ = 0; return true; default: - bro_analyzer()->ProtocolViolation(fmt("Invalid version in DTLS connection. Packet reported version: %d", version)); + invalid_version_count_++; + + if ( bro_analyzer()->ProtocolConfirmed() ) + { + reported_errors_++; + if ( reported_errors_ <= BifConst::SSL::dtls_max_reported_version_errors ) + bro_analyzer()->ProtocolViolation(fmt("Invalid version in DTLS connection. Packet reported version: %d", version)); + } + + if ( invalid_version_count_ > BifConst::SSL::dtls_max_version_errors ) + bro_analyzer()->SetSkip(true); return false; } %} diff --git a/src/analyzer/protocol/ssl/dtls.pac b/src/analyzer/protocol/ssl/dtls.pac index b08dd61f8f..b2aa34d5c5 100644 --- a/src/analyzer/protocol/ssl/dtls.pac +++ b/src/analyzer/protocol/ssl/dtls.pac @@ -10,6 +10,7 @@ namespace analyzer { namespace dtls { class DTLS_Analyzer; } } typedef analyzer::dtls::DTLS_Analyzer* DTLSAnalyzer; #include "DTLS.h" +#include "consts.bif.h" %} extern type DTLSAnalyzer; diff --git a/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log b/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log index 4eeaa4b07b..bd24bf02aa 100644 --- a/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log +++ b/testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log @@ -3,7 +3,7 @@ #empty_field (empty) #unset_field - #path loaded_scripts -#open 2018-06-08-16-37-15 +#open 2019-04-04-19-22-03 #fields name #types string scripts/base/init-bare.bro @@ -149,6 +149,7 @@ scripts/base/init-frameworks-and-bifs.bro build/scripts/base/bif/plugins/Bro_SSL.types.bif.bro build/scripts/base/bif/plugins/Bro_SSL.events.bif.bro build/scripts/base/bif/plugins/Bro_SSL.functions.bif.bro + build/scripts/base/bif/plugins/Bro_SSL.consts.bif.bro build/scripts/base/bif/plugins/Bro_SteppingStone.events.bif.bro build/scripts/base/bif/plugins/Bro_Syslog.events.bif.bro build/scripts/base/bif/plugins/Bro_TCP.events.bif.bro @@ -179,4 +180,4 @@ scripts/base/init-frameworks-and-bifs.bro build/scripts/base/bif/plugins/Bro_SQLiteWriter.sqlite.bif.bro scripts/policy/misc/loaded-scripts.bro scripts/base/utils/paths.bro -#close 2018-06-08-16-37-15 +#close 2019-04-04-19-22-03 diff --git a/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log b/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log index eaca1c489a..540910b350 100644 --- a/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log +++ b/testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log @@ -3,7 +3,7 @@ #empty_field (empty) #unset_field - #path loaded_scripts -#open 2018-09-05-20-33-08 +#open 2019-04-04-19-22-06 #fields name #types string scripts/base/init-bare.bro @@ -149,6 +149,7 @@ scripts/base/init-frameworks-and-bifs.bro build/scripts/base/bif/plugins/Bro_SSL.types.bif.bro build/scripts/base/bif/plugins/Bro_SSL.events.bif.bro build/scripts/base/bif/plugins/Bro_SSL.functions.bif.bro + build/scripts/base/bif/plugins/Bro_SSL.consts.bif.bro build/scripts/base/bif/plugins/Bro_SteppingStone.events.bif.bro build/scripts/base/bif/plugins/Bro_Syslog.events.bif.bro build/scripts/base/bif/plugins/Bro_TCP.events.bif.bro @@ -373,4 +374,4 @@ scripts/base/init-default.bro scripts/base/misc/find-filtered-trace.bro scripts/base/misc/version.bro scripts/policy/misc/loaded-scripts.bro -#close 2018-09-05-20-33-08 +#close 2019-04-04-19-22-06 diff --git a/testing/btest/Baseline/plugins.hooks/output b/testing/btest/Baseline/plugins.hooks/output index d4a84a5223..04908bed0b 100644 --- a/testing/btest/Baseline/plugins.hooks/output +++ b/testing/btest/Baseline/plugins.hooks/output @@ -277,7 +277,7 @@ 0.000000 MetaHookPost CallFunction(Log::__create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) -> 0.000000 MetaHookPost CallFunction(Log::__create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) -> 0.000000 MetaHookPost CallFunction(Log::__create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -> -0.000000 MetaHookPost CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) -> +0.000000 MetaHookPost CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) -> 0.000000 MetaHookPost CallFunction(Log::add_default_filter, , (Broker::LOG)) -> 0.000000 MetaHookPost CallFunction(Log::add_default_filter, , (Cluster::LOG)) -> 0.000000 MetaHookPost CallFunction(Log::add_default_filter, , (Config::LOG)) -> @@ -462,7 +462,7 @@ 0.000000 MetaHookPost CallFunction(Log::create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) -> 0.000000 MetaHookPost CallFunction(Log::create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) -> 0.000000 MetaHookPost CallFunction(Log::create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -> -0.000000 MetaHookPost CallFunction(Log::write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) -> +0.000000 MetaHookPost CallFunction(Log::write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) -> 0.000000 MetaHookPost CallFunction(NetControl::check_plugins, , ()) -> 0.000000 MetaHookPost CallFunction(NetControl::init, , ()) -> 0.000000 MetaHookPost CallFunction(Notice::want_pp, , ()) -> @@ -678,6 +678,7 @@ 0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SQLiteWriter.sqlite.bif.bro) -> -1 0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SSH.events.bif.bro) -> -1 0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SSH.types.bif.bro) -> -1 +0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SSL.consts.bif.bro) -> -1 0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SSL.events.bif.bro) -> -1 0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SSL.functions.bif.bro) -> -1 0.000000 MetaHookPost LoadFile(0, .<...>/Bro_SSL.types.bif.bro) -> -1 @@ -1179,7 +1180,7 @@ 0.000000 MetaHookPre CallFunction(Log::__create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) 0.000000 MetaHookPre CallFunction(Log::__create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) 0.000000 MetaHookPre CallFunction(Log::__create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -0.000000 MetaHookPre CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) +0.000000 MetaHookPre CallFunction(Log::__write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) 0.000000 MetaHookPre CallFunction(Log::add_default_filter, , (Broker::LOG)) 0.000000 MetaHookPre CallFunction(Log::add_default_filter, , (Cluster::LOG)) 0.000000 MetaHookPre CallFunction(Log::add_default_filter, , (Config::LOG)) @@ -1364,7 +1365,7 @@ 0.000000 MetaHookPre CallFunction(Log::create_stream, , (Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird])) 0.000000 MetaHookPre CallFunction(Log::create_stream, , (X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509])) 0.000000 MetaHookPre CallFunction(Log::create_stream, , (mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql])) -0.000000 MetaHookPre CallFunction(Log::write, , (PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T])) +0.000000 MetaHookPre CallFunction(Log::write, , (PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T])) 0.000000 MetaHookPre CallFunction(NetControl::check_plugins, , ()) 0.000000 MetaHookPre CallFunction(NetControl::init, , ()) 0.000000 MetaHookPre CallFunction(Notice::want_pp, , ()) @@ -1580,6 +1581,7 @@ 0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SQLiteWriter.sqlite.bif.bro) 0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SSH.events.bif.bro) 0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SSH.types.bif.bro) +0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SSL.consts.bif.bro) 0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SSL.events.bif.bro) 0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SSL.functions.bif.bro) 0.000000 MetaHookPre LoadFile(0, .<...>/Bro_SSL.types.bif.bro) @@ -2080,7 +2082,7 @@ 0.000000 | HookCallFunction Log::__create_stream(Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird]) 0.000000 | HookCallFunction Log::__create_stream(X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509]) 0.000000 | HookCallFunction Log::__create_stream(mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql]) -0.000000 | HookCallFunction Log::__write(PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T]) +0.000000 | HookCallFunction Log::__write(PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T]) 0.000000 | HookCallFunction Log::add_default_filter(Broker::LOG) 0.000000 | HookCallFunction Log::add_default_filter(Cluster::LOG) 0.000000 | HookCallFunction Log::add_default_filter(Config::LOG) @@ -2265,7 +2267,7 @@ 0.000000 | HookCallFunction Log::create_stream(Weird::LOG, [columns=Weird::Info, ev=Weird::log_weird, path=weird]) 0.000000 | HookCallFunction Log::create_stream(X509::LOG, [columns=X509::Info, ev=X509::log_x509, path=x509]) 0.000000 | HookCallFunction Log::create_stream(mysql::LOG, [columns=MySQL::Info, ev=MySQL::log_mysql, path=mysql]) -0.000000 | HookCallFunction Log::write(PacketFilter::LOG, [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T]) +0.000000 | HookCallFunction Log::write(PacketFilter::LOG, [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T]) 0.000000 | HookCallFunction NetControl::check_plugins() 0.000000 | HookCallFunction NetControl::init() 0.000000 | HookCallFunction Notice::want_pp() @@ -2481,6 +2483,7 @@ 0.000000 | HookLoadFile .<...>/Bro_SQLiteWriter.sqlite.bif.bro 0.000000 | HookLoadFile .<...>/Bro_SSH.events.bif.bro 0.000000 | HookLoadFile .<...>/Bro_SSH.types.bif.bro +0.000000 | HookLoadFile .<...>/Bro_SSL.consts.bif.bro 0.000000 | HookLoadFile .<...>/Bro_SSL.events.bif.bro 0.000000 | HookLoadFile .<...>/Bro_SSL.functions.bif.bro 0.000000 | HookLoadFile .<...>/Bro_SSL.types.bif.bro @@ -2699,7 +2702,7 @@ 0.000000 | HookLoadFile base<...>/x509 0.000000 | HookLoadFile base<...>/xmpp 0.000000 | HookLogInit packet_filter 1/1 {ts (time), node (string), filter (string), init (bool), success (bool)} -0.000000 | HookLogWrite packet_filter [ts=1552701731.192609, node=bro, filter=ip or not ip, init=T, success=T] +0.000000 | HookLogWrite packet_filter [ts=1554405757.770254, node=bro, filter=ip or not ip, init=T, success=T] 0.000000 | HookQueueEvent NetControl::init() 0.000000 | HookQueueEvent bro_init() 0.000000 | HookQueueEvent filter_change_tracking() diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.dtls-no-dtls/.stdout b/testing/btest/Baseline/scripts.base.protocols.ssl.dtls-no-dtls/.stdout new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test b/testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test new file mode 100644 index 0000000000..c8721529c9 --- /dev/null +++ b/testing/btest/scripts/base/protocols/ssl/dtls-no-dtls.test @@ -0,0 +1,15 @@ +# This tests checks that non-dtls connections to which we attach don't trigger tons of errors. + +# @TEST-EXEC: bro -C -r $TRACES/dns-txt-multiple.trace %INPUT +# @TEST-EXEC: btest-diff .stdout + +event bro_init() + { + const add_ports = { 53/udp }; + Analyzer::register_for_ports(Analyzer::ANALYZER_DTLS, add_ports); + } + +event protocol_violation(c: connection, atype: Analyzer::Tag, aid: count, reason: string) + { + print c$id, atype, reason; + } From 5e9b119a086f5311d5462d86615732eb23ddc65e Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Apr 2019 12:35:50 -0700 Subject: [PATCH 06/13] Use a default binpac flowbuffer policy --- aux/binpac | 2 +- src/main.cc | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/aux/binpac b/aux/binpac index bb2476465e..ce89e30109 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit bb2476465e304a00c368bd73d40cc6f734be5311 +Subproject commit ce89e301091fd8fd6ef53701b7b29a79d888b637 diff --git a/src/main.cc b/src/main.cc index 473f3a72e7..09b1ebfaeb 100644 --- a/src/main.cc +++ b/src/main.cc @@ -893,7 +893,11 @@ int main(int argc, char** argv) // Must come after plugin activation (and also after hash // initialization). - binpac::init(); + binpac::FlowBuffer::Policy flowbuffer_policy; + flowbuffer_policy.max_capacity = 10 * 1024 * 1024; + flowbuffer_policy.min_capacity = 512; + flowbuffer_policy.contract_threshold = 2 * 1024 * 1024; + binpac::init(&flowbuffer_policy); init_event_handlers(); From fe044ecc903e14b4520f21b9fc85def72e094af7 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Apr 2019 12:42:29 -0700 Subject: [PATCH 07/13] Set PE analyzer CMake dependencies correctly --- src/file_analysis/analyzer/pe/CMakeLists.txt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/file_analysis/analyzer/pe/CMakeLists.txt b/src/file_analysis/analyzer/pe/CMakeLists.txt index 7fc89bfd51..5708f98e8f 100644 --- a/src/file_analysis/analyzer/pe/CMakeLists.txt +++ b/src/file_analysis/analyzer/pe/CMakeLists.txt @@ -6,5 +6,12 @@ include_directories(BEFORE ${CMAKE_CURRENT_SOURCE_DIR} bro_plugin_begin(Bro PE) bro_plugin_cc(PE.cc Plugin.cc) bro_plugin_bif(events.bif) -bro_plugin_pac(pe.pac pe-file.pac pe-analyzer.pac) +bro_plugin_pac( + pe.pac + pe-analyzer.pac + pe-file-headers.pac + pe-file-idata.pac + pe-file.pac + pe-file-types.pac +) bro_plugin_end() From c94358f1aa4a60cd75aa8f6faf89b60b7fdd9006 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Apr 2019 12:43:47 -0700 Subject: [PATCH 08/13] Improve PE file analysis * Consider parsing done after processing headers * Remove the analyzer when done parsing * Enforce a maximum DOS stub program length (helps filter out non-PE) --- src/file_analysis/analyzer/pe/PE.cc | 5 +++-- src/file_analysis/analyzer/pe/pe-file-headers.pac | 10 +++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/file_analysis/analyzer/pe/PE.cc b/src/file_analysis/analyzer/pe/PE.cc index 9db13291b0..070aff32dd 100644 --- a/src/file_analysis/analyzer/pe/PE.cc +++ b/src/file_analysis/analyzer/pe/PE.cc @@ -20,7 +20,8 @@ PE::~PE() bool PE::DeliverStream(const u_char* data, uint64 len) { if ( conn->is_done() ) - return true; + return false; + try { interp->NewData(data, data + len); @@ -30,7 +31,7 @@ bool PE::DeliverStream(const u_char* data, uint64 len) return false; } - return true; + return ! conn->is_done(); } bool PE::EndOfFile() diff --git a/src/file_analysis/analyzer/pe/pe-file-headers.pac b/src/file_analysis/analyzer/pe/pe-file-headers.pac index f12d76e035..9eee6e03da 100644 --- a/src/file_analysis/analyzer/pe/pe-file-headers.pac +++ b/src/file_analysis/analyzer/pe/pe-file-headers.pac @@ -1,3 +1,8 @@ +# Do not try parsing if the DOS stub program seems larger than 4mb. +# DOS stub programs are not expected to be much more than on the order of +# hundreds of bytes even though the format allows a full 32-bit range. +let MAX_DOS_CODE_LENGTH = 4 * 1024 * 1024; + type Headers = record { dos_header : DOS_Header; dos_code : DOS_Code(dos_code_len); @@ -6,6 +11,9 @@ type Headers = record { } &let { dos_code_len: uint32 = dos_header.AddressOfNewExeHeader > 64 ? dos_header.AddressOfNewExeHeader - 64 : 0; length: uint64 = 64 + dos_code_len + pe_header.length + section_headers.length; + + # Do not care about parsing rest of the file so mark done now ... + proc: bool = $context.connection.mark_done(); }; # The DOS header gives us the offset of the NT headers @@ -28,7 +36,7 @@ type DOS_Header = record { OEMid : uint16; OEMinfo : uint16; Reserved2 : uint16[10]; - AddressOfNewExeHeader : uint32; + AddressOfNewExeHeader : uint32 &enforce(AddressOfNewExeHeader >= 64 && (AddressOfNewExeHeader - 64) < MAX_DOS_CODE_LENGTH); } &length=64; type DOS_Code(len: uint32) = record { From 23c244d448de2b936797de4adeddf12b7135af58 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Apr 2019 17:06:03 -0700 Subject: [PATCH 09/13] Change next version number in NEWS --- NEWS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 13f05baa3b..bde87d6f55 100644 --- a/NEWS +++ b/NEWS @@ -3,8 +3,8 @@ This document summarizes the most important changes in the current Bro release. For an exhaustive list of changes, see the ``CHANGES`` file (note that submodules, such as Broker, come with their own ``CHANGES``.) -Bro 2.7 -======= +Zeek 3.0.0 +========== New Functionality ----------------- From 9c843a7d83c4a7f658f68a9ef2627ff8d34171bf Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 5 Apr 2019 17:06:26 -0700 Subject: [PATCH 10/13] Add script to update external test repo commit pointers It will prompt to update the file storing the external test repo commit hash when a change is detected upon running update-changes. --- .update-changes.cfg | 12 +---- CHANGES | 4 ++ VERSION | 2 +- .../scripts/update-external-repo-pointer.sh | 49 +++++++++++++++++++ 4 files changed, 56 insertions(+), 11 deletions(-) create mode 100755 testing/scripts/update-external-repo-pointer.sh diff --git a/.update-changes.cfg b/.update-changes.cfg index e3d04b7422..ed23fb4565 100644 --- a/.update-changes.cfg +++ b/.update-changes.cfg @@ -7,15 +7,7 @@ function new_version_hook # test suite repos to check out on a CI system. version=$1 - if [ -d testing/external/zeek-testing ]; then - echo "Updating testing/external/commit-hash.zeek-testing" - ( cd testing/external/zeek-testing && git fetch origin && git rev-parse origin/master ) > testing/external/commit-hash.zeek-testing - git add testing/external/commit-hash.zeek-testing - fi + ./testing/scripts/update-external-repo-pointer.sh testing/external/zeek-testing testing/external/commit-hash.zeek-testing - if [ -d testing/external/zeek-testing-private ]; then - echo "Updating testing/external/commit-hash.zeek-testing-private" - ( cd testing/external/zeek-testing-private && git fetch origin && git rev-parse origin/master ) > testing/external/commit-hash.zeek-testing-private - git add testing/external/commit-hash.zeek-testing-private - fi + ./testing/scripts/update-external-repo-pointer.sh testing/external/zeek-testing-private testing/external/commit-hash.zeek-testing-private } diff --git a/CHANGES b/CHANGES index e0be717f10..43fa7e3ad7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,8 @@ +2.6-205 | 2019-04-05 17:06:26 -0700 + + * Add script to update external test repo commit pointers (Jon Siwek, Corelight) + 2.6-203 | 2019-04-04 16:35:52 -0700 * Update DTLS error handling (Johanna Amann, Corelight) diff --git a/VERSION b/VERSION index 92df2bb961..a0c99ee2be 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.6-203 +2.6-205 diff --git a/testing/scripts/update-external-repo-pointer.sh b/testing/scripts/update-external-repo-pointer.sh new file mode 100755 index 0000000000..e6711a0d9d --- /dev/null +++ b/testing/scripts/update-external-repo-pointer.sh @@ -0,0 +1,49 @@ +#! /usr/bin/env bash + +set -e + +if [ $# -ne 2 ]; then + echo "usage: $0 " + exit 1 +fi + +repo_dir=$1 +hash_file=$2 + +repo_base=$(basename $repo_dir) +file_base=$(basename $hash_file) + +if [ ! -d "$repo_dir" ]; then + echo "External repo does not exist: $repo_dir" + exit 1 +fi + +printf "Checking for '$repo_base' changes ..." + +origin_hash=$(cd $repo_dir && git fetch origin && git rev-parse origin/master) +head_hash=$(cd $repo_dir && git rev-parse HEAD) +file_hash=$(cat $hash_file) + +if [ "$file_hash" != "$head_hash" ]; then + printf "\n" + printf "\n" + printf " '$repo_base' pointer has changed:\n" + + line=" $file_base at $file_hash" + len=${#line} + + printf "%${len}s\n" "$line" + printf "%${len}s\n" "origin/master at $origin_hash" + printf "%${len}s\n" "HEAD at $head_hash" + printf "\n" + printf "Update file '$file_base' to HEAD commit ? " + + read -p "[Y/n] " choice + + case "$choice" in + n|N) echo "Skipped '$repo_base'";; + *) echo $head_hash > $hash_file && git add $hash_file && echo "Updated '$file_base'";; + esac +else + echo " none" +fi From 0c508f828016922e5897d27a826c9de796138c70 Mon Sep 17 00:00:00 2001 From: Mauro Palumbo Date: Mon, 8 Apr 2019 22:32:14 +0200 Subject: [PATCH 11/13] smb2_write_response event added --- src/analyzer/protocol/smb/smb2-com-write.pac | 9 +++++++++ src/analyzer/protocol/smb/smb2_com_write.bif | 15 +++++++++++++++ .../.stdout | 0 .../base/protocols/smb/smb2-write-response.test | 13 +++++++++++++ 4 files changed, 37 insertions(+) create mode 100644 testing/btest/Baseline/scripts.base.protocols.smb.smb2-write-response/.stdout create mode 100644 testing/btest/scripts/base/protocols/smb/smb2-write-response.test diff --git a/src/analyzer/protocol/smb/smb2-com-write.pac b/src/analyzer/protocol/smb/smb2-com-write.pac index 177a3a84bd..c117fc793d 100644 --- a/src/analyzer/protocol/smb/smb2-com-write.pac +++ b/src/analyzer/protocol/smb/smb2-com-write.pac @@ -24,6 +24,15 @@ refine connection SMB_Conn += { function proc_smb2_write_response(h: SMB2_Header, val: SMB2_write_response) : bool %{ + + if ( smb2_write_response ) + { + BifEvent::generate_smb2_write_response(bro_analyzer(), + bro_analyzer()->Conn(), + BuildSMB2HeaderVal(h), + ${val.write_count}); + } + return true; %} diff --git a/src/analyzer/protocol/smb/smb2_com_write.bif b/src/analyzer/protocol/smb/smb2_com_write.bif index 90efce049c..66dab9b077 100644 --- a/src/analyzer/protocol/smb/smb2_com_write.bif +++ b/src/analyzer/protocol/smb/smb2_com_write.bif @@ -16,3 +16,18 @@ ## ## .. bro:see:: smb2_message event smb2_write_request%(c: connection, hdr: SMB2::Header, file_id: SMB2::GUID, offset: count, length: count%); + +## Generated for :abbr:`SMB (Server Message Block)`/:abbr:`CIFS (Common Internet File System)` +## version 2 requests of type *write*. This is sent by the server in response to a write request or +## named pipe on the server. +## +## For more information, see MS-SMB2:2.2.22 +## +## c: The connection. +## +## hdr: The parsed header of the :abbr:`SMB (Server Message Block)` version 2 message. +## +## length: The number of bytes of the file being written. +## +## .. bro:see:: smb2_message +event smb2_write_response%(c: connection, hdr: SMB2::Header, length: count%); diff --git a/testing/btest/Baseline/scripts.base.protocols.smb.smb2-write-response/.stdout b/testing/btest/Baseline/scripts.base.protocols.smb.smb2-write-response/.stdout new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/scripts/base/protocols/smb/smb2-write-response.test b/testing/btest/scripts/base/protocols/smb/smb2-write-response.test new file mode 100644 index 0000000000..68c9ad47f6 --- /dev/null +++ b/testing/btest/scripts/base/protocols/smb/smb2-write-response.test @@ -0,0 +1,13 @@ +# @TEST-EXEC: bro -r $TRACES/smb/smb2readwrite.pcap %INPUT +# @TEST-EXEC: btest-diff .stdout + +@load base/protocols/smb + +# A test for write response. +event smb2_write_response(c: connection, hdr: SMB2::Header, length: count) + { + print fmt("smb2_write_response %s -> %s:%d, length: %d", c$id$orig_h, c$id$resp_h, c$id$resp_p, length); + print (hdr); + } + + From 1c7e41e5067f2cbb51ea1612064993dfa28a06db Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 12 Apr 2019 13:21:10 -0700 Subject: [PATCH 12/13] Updating submodule(s). [nomail] --- aux/broker | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aux/broker b/aux/broker index 7dab576984..de0c8e0ece 160000 --- a/aux/broker +++ b/aux/broker @@ -1 +1 @@ -Subproject commit 7dab576984dee1f58fe5ceb81f36b63128d58860 +Subproject commit de0c8e0ecea39dd556a16f4ecc0d482e936c38ac From f96bc81f8599b2733f378b3a7edf5b062a88e648 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 12 Apr 2019 16:44:14 -0700 Subject: [PATCH 13/13] Updating submodule(s). [nomail] --- aux/broctl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aux/broctl b/aux/broctl index afc0260abf..a49144d3dd 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit afc0260abf663f4b44d535d66d378fde7b0d5206 +Subproject commit a49144d3dd26d906ad906ace97db3d093c510142