diff --git a/CHANGES b/CHANGES index 7b825ee3de..d91a2e4700 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,30 @@ +2.2-341 | 2014-04-17 18:01:01 -0500 + + * Refactor initialization of ASCII log writer options. (Jon Siwek) + + * Fix a memory leak in ASCII log writer. (Jon Siwek) + +2.2-338 | 2014-04-17 17:48:17 -0500 + + * Disable input/logging threads setting their names on every + heartbeat. (Jon Siwek) + + * Fix bug when clearing Bloom filter contents. Reported by + @colonelxc. (Matthias Vallentin) + +2.2-335 | 2014-04-10 15:04:57 -0700 + + * Small logic fix for main SSL script. (Bernhard Amann) + + * Update DPD signatures for detecting TLS 1.2. (Bernhard Amann) + + * Remove unused data member of SMTP_Analyzer to silence a Coverity + warning. (Jon Siwek) + + * Fix missing @load dependencies in some scripts. Also update the + unit test which is supposed to catch such errors. (Jon Siwek) + 2.2-326 | 2014-04-08 15:21:51 -0700 * Add SNMP datagram parsing support.This supports parsing of SNMPv1 diff --git a/VERSION b/VERSION index c18ef554a5..85d7fee81b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.2-326 +2.2-341 diff --git a/aux/broctl b/aux/broctl index f249570e3f..d99150801b 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit f249570e3fb4c83e532cc0813786f0ff60c4dea9 +Subproject commit d99150801b7844e082b5421d1efe4050702d350e diff --git a/scripts/base/protocols/ssl/main.bro b/scripts/base/protocols/ssl/main.bro index 75f7ad7364..5b974222a1 100644 --- a/scripts/base/protocols/ssl/main.bro +++ b/scripts/base/protocols/ssl/main.bro @@ -136,8 +136,10 @@ function finish(c: connection, remove_analyzer: bool) { log_record(c$ssl); if ( remove_analyzer && disable_analyzer_after_detection && c?$ssl && c$ssl?$analyzer_id ) + { disable_analyzer(c$id, c$ssl$analyzer_id); delete c$ssl$analyzer_id; + } } event ssl_client_hello(c: connection, version: count, possible_ts: time, client_random: string, session_id: string, ciphers: index_vec) &priority=5 diff --git a/src/input/ReaderBackend.cc b/src/input/ReaderBackend.cc index abf369dd54..4c7540609c 100644 --- a/src/input/ReaderBackend.cc +++ b/src/input/ReaderBackend.cc @@ -215,6 +215,8 @@ bool ReaderBackend::Init(const int arg_num_fields, if ( Failed() ) return true; + SetOSName(Fmt("bro: %s", Name())); + num_fields = arg_num_fields; fields = arg_fields; diff --git a/src/logging/WriterBackend.cc b/src/logging/WriterBackend.cc index 5e4230c8e3..35d3137c76 100644 --- a/src/logging/WriterBackend.cc +++ b/src/logging/WriterBackend.cc @@ -180,6 +180,7 @@ void WriterBackend::DisableFrontend() bool WriterBackend::Init(int arg_num_fields, const Field* const* arg_fields) { + SetOSName(Fmt("bro: %s", Name())); num_fields = arg_num_fields; fields = arg_fields; diff --git a/src/logging/writers/Ascii.cc b/src/logging/writers/Ascii.cc index b926a6c55b..43ffe47308 100644 --- a/src/logging/writers/Ascii.cc +++ b/src/logging/writers/Ascii.cc @@ -24,43 +24,13 @@ Ascii::Ascii(WriterFrontend* frontend) : WriterBackend(frontend) tsv = false; use_json = false; formatter = 0; + + InitConfigOptions(); + init_options = InitFilterOptions(); } -Ascii::~Ascii() +void Ascii::InitConfigOptions() { - if ( ! ascii_done ) - { - fprintf(stderr, "internal error: finish missing\n"); - abort(); - } - - delete formatter; - } - -bool Ascii::WriteHeaderField(const string& key, const string& val) - { - string str = meta_prefix + key + separator + val + "\n"; - - return safe_write(fd, str.c_str(), str.length()); - } - -void Ascii::CloseFile(double t) - { - if ( ! fd ) - return; - - if ( include_meta && ! tsv ) - WriteHeaderField("close", Timestamp(0)); - - safe_close(fd); - fd = 0; - } - -bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * fields) - { - assert(! fd); - - // Set some default values. output_to_stdout = BifConst::LogAscii::output_to_stdout; include_meta = BifConst::LogAscii::include_meta; use_json = BifConst::LogAscii::use_json; @@ -96,9 +66,15 @@ bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * (const char*) tsfmt.Bytes(), tsfmt.Len() ); + } + +bool Ascii::InitFilterOptions() + { + const WriterInfo& info = Info(); // Set per-filter configuration options. - for ( WriterInfo::config_map::const_iterator i = info.config.begin(); i != info.config.end(); i++ ) + for ( WriterInfo::config_map::const_iterator i = info.config.begin(); + i != info.config.end(); ++i ) { if ( strcmp(i->first, "tsv") == 0 ) { @@ -158,6 +134,17 @@ bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * json_timestamps.assign(i->second); } + if ( ! InitFormatter() ) + return false; + + return true; + } + +bool Ascii::InitFormatter() + { + delete formatter; + formatter = 0; + if ( use_json ) { formatter::JSON::TimeFormat tf = formatter::JSON::TS_EPOCH; @@ -179,7 +166,6 @@ bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * // Using JSON implicitly turns off the header meta fields. include_meta = false; } - else { // Use the default "Bro logs" format. @@ -189,6 +175,46 @@ bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * formatter = new formatter::Ascii(this, sep_info); } + return true; + } + +Ascii::~Ascii() + { + if ( ! ascii_done ) + { + fprintf(stderr, "internal error: finish missing\n"); + abort(); + } + + delete formatter; + } + +bool Ascii::WriteHeaderField(const string& key, const string& val) + { + string str = meta_prefix + key + separator + val + "\n"; + + return safe_write(fd, str.c_str(), str.length()); + } + +void Ascii::CloseFile(double t) + { + if ( ! fd ) + return; + + if ( include_meta && ! tsv ) + WriteHeaderField("close", Timestamp(0)); + + safe_close(fd); + fd = 0; + } + +bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * fields) + { + assert(! fd); + + if ( ! init_options ) + return false; + string path = info.path; if ( output_to_stdout ) @@ -206,58 +232,65 @@ bool Ascii::DoInit(const WriterInfo& info, int num_fields, const Field* const * return false; } - if ( include_meta ) + if ( ! WriteHeader(path) ) { - string names; - string types; - - for ( int i = 0; i < num_fields; ++i ) - { - if ( i > 0 ) - { - names += separator; - types += separator; - } - - names += string(fields[i]->name); - types += fields[i]->TypeName().c_str(); - } - - if ( tsv ) - { - // A single TSV-style line is all we need. - string str = names + "\n"; - if ( ! safe_write(fd, str.c_str(), str.length()) ) - goto write_error; - - return true; - } - - string str = meta_prefix - + "separator " // Always use space as separator here. - + get_escaped_string(separator, false) - + "\n"; - - if ( ! safe_write(fd, str.c_str(), str.length()) ) - goto write_error; - - if ( ! (WriteHeaderField("set_separator", get_escaped_string(set_separator, false)) && - WriteHeaderField("empty_field", get_escaped_string(empty_field, false)) && - WriteHeaderField("unset_field", get_escaped_string(unset_field, false)) && - WriteHeaderField("path", get_escaped_string(path, false)) && - WriteHeaderField("open", Timestamp(0))) ) - goto write_error; - - if ( ! (WriteHeaderField("fields", names) - && WriteHeaderField("types", types)) ) - goto write_error; + Error(Fmt("error writing to %s: %s", fname.c_str(), Strerror(errno))); + return false; } return true; + } -write_error: - Error(Fmt("error writing to %s: %s", fname.c_str(), Strerror(errno))); - return false; +bool Ascii::WriteHeader(const string& path) + { + if ( ! include_meta ) + return true; + + string names; + string types; + + for ( int i = 0; i < NumFields(); ++i ) + { + if ( i > 0 ) + { + names += separator; + types += separator; + } + + names += string(Fields()[i]->name); + types += Fields()[i]->TypeName().c_str(); + } + + if ( tsv ) + { + // A single TSV-style line is all we need. + string str = names + "\n"; + if ( ! safe_write(fd, str.c_str(), str.length()) ) + return false; + + return true; + } + + string str = meta_prefix + + "separator " // Always use space as separator here. + + get_escaped_string(separator, false) + + "\n"; + + if ( ! safe_write(fd, str.c_str(), str.length()) ) + return false; + + if ( ! (WriteHeaderField("set_separator", get_escaped_string(set_separator, false)) && + WriteHeaderField("empty_field", get_escaped_string(empty_field, false)) && + WriteHeaderField("unset_field", get_escaped_string(unset_field, false)) && + WriteHeaderField("path", get_escaped_string(path, false)) && + WriteHeaderField("open", Timestamp(0))) ) + return false; + + if ( ! (WriteHeaderField("fields", names) && + WriteHeaderField("types", types)) ) + return false; + + return true; } bool Ascii::DoFlush(double network_time) @@ -294,7 +327,7 @@ bool Ascii::DoWrite(int num_fields, const Field* const * fields, if ( ! formatter->Describe(&desc, num_fields, fields, vals) ) return false; - desc.AddRaw("\n"); + desc.AddRaw("\n", 1); const char* bytes = (const char*)desc.Bytes(); int len = desc.Len(); diff --git a/src/logging/writers/Ascii.h b/src/logging/writers/Ascii.h index 15afdef62f..54402cc141 100644 --- a/src/logging/writers/Ascii.h +++ b/src/logging/writers/Ascii.h @@ -34,9 +34,13 @@ protected: private: bool IsSpecial(string path) { return path.find("/dev/") == 0; } + bool WriteHeader(const string& path); bool WriteHeaderField(const string& key, const string& value); void CloseFile(double t); string Timestamp(double t); // Uses current time if t is zero. + void InitConfigOptions(); + bool InitFilterOptions(); + bool InitFormatter(); int fd; string fname; @@ -58,6 +62,7 @@ private: string json_timestamps; threading::formatter::Formatter* formatter; + bool init_options; }; } diff --git a/src/probabilistic/BloomFilter.cc b/src/probabilistic/BloomFilter.cc index f6e4017626..ef671268b9 100644 --- a/src/probabilistic/BloomFilter.cc +++ b/src/probabilistic/BloomFilter.cc @@ -72,7 +72,7 @@ bool BasicBloomFilter::Empty() const void BasicBloomFilter::Clear() { - bits->Clear(); + bits->Reset(); } bool BasicBloomFilter::Merge(const BloomFilter* other) @@ -190,7 +190,7 @@ bool CountingBloomFilter::Empty() const void CountingBloomFilter::Clear() { - cells->Clear(); + cells->Reset(); } bool CountingBloomFilter::Merge(const BloomFilter* other) diff --git a/src/probabilistic/CounterVector.cc b/src/probabilistic/CounterVector.cc index 317a28d851..8608015422 100644 --- a/src/probabilistic/CounterVector.cc +++ b/src/probabilistic/CounterVector.cc @@ -75,9 +75,9 @@ bool CounterVector::AllZero() const return bits->AllZero(); } -void CounterVector::Clear() +void CounterVector::Reset() { - bits->Clear(); + bits->Reset(); } CounterVector::count_type CounterVector::Count(size_type cell) const diff --git a/src/probabilistic/CounterVector.h b/src/probabilistic/CounterVector.h index d3efd1aa31..247a646eb1 100644 --- a/src/probabilistic/CounterVector.h +++ b/src/probabilistic/CounterVector.h @@ -86,7 +86,7 @@ public: /** * Sets all counters to 0. */ - void Clear(); + void Reset(); /** * Retrieves the number of cells in the storage. diff --git a/src/threading/MsgThread.cc b/src/threading/MsgThread.cc index c713f65986..c603f20625 100644 --- a/src/threading/MsgThread.cc +++ b/src/threading/MsgThread.cc @@ -53,7 +53,6 @@ public: { network_time = arg_network_time; current_time = arg_current_time; } virtual bool Process() { - Object()->HeartbeatInChild(); return Object()->OnHeartbeat(network_time, current_time); } @@ -254,15 +253,6 @@ void MsgThread::Heartbeat() SendIn(new HeartbeatMessage(this, network_time, current_time())); } -void MsgThread::HeartbeatInChild() - { - string n = Fmt("bro: %s (%" PRIu64 "/%" PRIu64 ")", Name(), - cnt_sent_in - queue_in.Size(), - cnt_sent_out - queue_out.Size()); - - SetOSName(n.c_str()); - } - void MsgThread::Finished() { child_finished = true; diff --git a/src/threading/MsgThread.h b/src/threading/MsgThread.h index c5ba5b676f..8b63d91c60 100644 --- a/src/threading/MsgThread.h +++ b/src/threading/MsgThread.h @@ -197,10 +197,6 @@ protected: */ virtual void Heartbeat(); - /** Internal heartbeat processing. Called from child. - */ - void HeartbeatInChild(); - /** Returns true if a child command has reported a failure. In that case, we'll * be in the process of killing this thread and no further activity * should carried out. To be called only from this child thread. diff --git a/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout b/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout new file mode 100644 index 0000000000..b59ed28b18 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssl.dpd/.stdout @@ -0,0 +1,8 @@ +Start test run +Client hello, 192.168.4.149, 91.227.4.92, 2 +Start test run +Client hello, 192.150.187.164, 194.127.84.106, 2 +Client hello, 192.150.187.164, 194.127.84.106, 769 +Client hello, 192.150.187.164, 194.127.84.106, 769 +Start test run +Client hello, 10.0.0.80, 68.233.76.12, 771 diff --git a/testing/btest/Traces/tls/ssl-v2.trace b/testing/btest/Traces/tls/ssl-v2.trace new file mode 100644 index 0000000000..a97ea3fa15 Binary files /dev/null and b/testing/btest/Traces/tls/ssl-v2.trace differ diff --git a/testing/btest/core/leaks/ascii-log-rotation.bro b/testing/btest/core/leaks/ascii-log-rotation.bro new file mode 100644 index 0000000000..a84f80ea90 --- /dev/null +++ b/testing/btest/core/leaks/ascii-log-rotation.bro @@ -0,0 +1,75 @@ +# Needs perftools support. +# +# @TEST-SERIALIZE: comm +# @TEST-GROUP: leaks +# +# @TEST-REQUIRES: bro --help 2>&1 | grep -q mem-leaks +# +# @TEST-EXEC: btest-bg-run receiver HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -b -m ../receiver.bro +# @TEST-EXEC: sleep 1 +# @TEST-EXEC: btest-bg-run sender HEAP_CHECK_DUMP_DIRECTORY=. HEAPCHECK=local bro -b -m ../sender.bro +# @TEST-EXEC: sleep 1 +# @TEST-EXEC: btest-bg-wait 60 + +@TEST-START-FILE sender.bro + +@load base/frameworks/communication +@load base/protocols/dns + +redef Communication::nodes += { + ["foo"] = [$host = 127.0.0.1, $connect=T] +}; + +global write_count: count = 0; + +event do_write() + { + print "do_write"; + local cid: conn_id = conn_id($orig_h=1.2.3.4,$orig_p=1/tcp, + $resp_h=5.6.7.8,$resp_p=2/tcp); + local dns_info_dummy = DNS::Info($ts=network_time(), $uid="FAKE", + $id=cid, $proto=tcp); + Log::write(DNS::LOG, dns_info_dummy); + schedule .1sec { do_write() }; + ++write_count; + + if ( write_count == 200 ) + terminate(); + } + +event remote_connection_handshake_done(p: event_peer) + { + print "remote_connection_handshake_done", p; + schedule .1sec { do_write() }; + } + +event remote_connection_closed(p: event_peer) + { + print "remote_connection_closed", p; + } + +@TEST-END-FILE + +@TEST-START-FILE receiver.bro + +@load frameworks/communication/listen +@load base/protocols/dns + +redef Communication::nodes += { + ["foo"] = [$host = 127.0.0.1, $connect=F, $request_logs=T] +}; + +redef Log::default_rotation_interval = 2sec; + +event remote_connection_handshake_done(p: event_peer) + { + print "remote_connection_handshake_done", p; + } + +event remote_connection_closed(p: event_peer) + { + print "remote_connection_closed", p; + terminate(); + } + +@TEST-END-FILE diff --git a/testing/btest/scripts/base/protocols/ssl/dpd.test b/testing/btest/scripts/base/protocols/ssl/dpd.test new file mode 100644 index 0000000000..ff1f6385ec --- /dev/null +++ b/testing/btest/scripts/base/protocols/ssl/dpd.test @@ -0,0 +1,19 @@ +# @TEST-EXEC: bro -C -b -r $TRACES/tls/ssl-v2.trace %INPUT +# @TEST-EXEC: bro -b -r $TRACES/tls/ssl.v3.trace %INPUT +# @TEST-EXEC: bro -b -r $TRACES/tls/tls1.2.trace %INPUT +# @TEST-EXEC: btest-diff .stdout + +@load base/frameworks/dpd +@load base/frameworks/signatures +@load-sigs base/protocols/ssl/dpd.sig + +event bro_init() + { + print "Start test run"; + } + +event ssl_client_hello(c: connection, version: count, possible_ts: time, client_random: string, session_id: string, ciphers: index_vec) &priority=5 + { + print "Client hello", c$id$orig_h, c$id$resp_h, version; + } +