From f6c9b69eda29913c51e09b51daa8ed5a3f416513 Mon Sep 17 00:00:00 2001 From: Bernhard Amann Date: Fri, 7 Sep 2012 10:57:52 -0700 Subject: [PATCH 1/7] reorder a few statements in scan.l to make 1.5msecs etc work. Adresses #872 --- src/scan.l | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/scan.l b/src/scan.l index 1b3d09f879..377c74cc1a 100644 --- a/src/scan.l +++ b/src/scan.l @@ -479,12 +479,6 @@ F RET_CONST(new Val(false, TYPE_BOOL)) RET_CONST(new PortVal(p, TRANSPORT_UNKNOWN)) } -({D}"."){3}{D} RET_CONST(new AddrVal(yytext)) - -"0x"{HEX}+ RET_CONST(new Val(static_cast(strtoull(yytext, 0, 16)), TYPE_COUNT)) - -{H}("."{H})+ RET_CONST(dns_mgr->LookupHost(yytext)) - {FLOAT}{OWS}day(s?) RET_CONST(new IntervalVal(atof(yytext),Days)) {FLOAT}{OWS}hr(s?) RET_CONST(new IntervalVal(atof(yytext),Hours)) {FLOAT}{OWS}min(s?) RET_CONST(new IntervalVal(atof(yytext),Minutes)) @@ -492,6 +486,12 @@ F RET_CONST(new Val(false, TYPE_BOOL)) {FLOAT}{OWS}msec(s?) RET_CONST(new IntervalVal(atof(yytext),Milliseconds)) {FLOAT}{OWS}usec(s?) RET_CONST(new IntervalVal(atof(yytext),Microseconds)) +({D}"."){3}{D} RET_CONST(new AddrVal(yytext)) + +"0x"{HEX}+ RET_CONST(new Val(static_cast(strtoull(yytext, 0, 16)), TYPE_COUNT)) + +{H}("."{H})+ RET_CONST(dns_mgr->LookupHost(yytext)) + \"([^\\\n\"]|{ESCSEQ})*\" { const char* text = yytext; int len = strlen(text) + 1; From 67d01ab9e9d1edebb8d7b19795fc07d3023d5b22 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 7 Sep 2012 15:15:48 -0500 Subject: [PATCH 2/7] Small change to non-blocking DNS initialization. The trailing dot on "localhost." circumvents use of /etc/hosts in some environments (I saw it on FreeBSD 9.0-RELEASE-p3) and so emits an actual DNS query. When running the test suite, that would be hundreds of useless queries. --- src/nb_dns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nb_dns.c b/src/nb_dns.c index d3b3c5c4de..3051be9bc2 100644 --- a/src/nb_dns.c +++ b/src/nb_dns.c @@ -124,7 +124,7 @@ nb_dns_init(char *errstr) nd->s = -1; /* XXX should be able to init static hostent struct some other way */ - (void)gethostbyname("localhost."); + (void)gethostbyname("localhost"); if ((_res.options & RES_INIT) == 0 && res_init() == -1) { snprintf(errstr, NB_DNS_ERRSIZE, "res_init() failed"); From bd84ff2c2051ff34a4b2060cce718875e23acf8c Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 7 Sep 2012 16:25:07 -0500 Subject: [PATCH 3/7] Adjusting some unit tests that do cluster communication. Added explicit synchronization and termination points to make the tests more reliable and exit earlier in most cases. --- .../base/frameworks/cluster/start-it-up.bro | 15 ++++++- .../base/frameworks/notice/cluster.bro | 37 +++++++++++++++-- .../frameworks/notice/suppression-cluster.bro | 40 +++++++++++++++++-- 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro b/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro index a1069d1bd0..89f8d6b168 100644 --- a/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro +++ b/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro @@ -5,7 +5,7 @@ # @TEST-EXEC: btest-bg-run proxy-2 BROPATH=$BROPATH:.. CLUSTER_NODE=proxy-2 bro %INPUT # @TEST-EXEC: btest-bg-run worker-1 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro %INPUT # @TEST-EXEC: btest-bg-run worker-2 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-2 bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 15 # @TEST-EXEC: btest-diff manager-1/.stdout # @TEST-EXEC: btest-diff proxy-1/.stdout # @TEST-EXEC: btest-diff proxy-2/.stdout @@ -22,7 +22,20 @@ redef Cluster::nodes = { }; @TEST-END-FILE +global peer_count = 0; + event remote_connection_handshake_done(p: event_peer) { print "Connected to a peer"; + if ( Cluster::node == "manager-1" ) + { + peer_count = peer_count + 1; + if ( peer_count == 4 ) + terminate_communication(); + } + } + +event remote_connection_closed(p: event_peer) + { + terminate(); } diff --git a/testing/btest/scripts/base/frameworks/notice/cluster.bro b/testing/btest/scripts/base/frameworks/notice/cluster.bro index 8d54a27eaf..47932edb8e 100644 --- a/testing/btest/scripts/base/frameworks/notice/cluster.bro +++ b/testing/btest/scripts/base/frameworks/notice/cluster.bro @@ -2,9 +2,9 @@ # # @TEST-EXEC: btest-bg-run manager-1 BROPATH=$BROPATH:.. CLUSTER_NODE=manager-1 bro %INPUT # @TEST-EXEC: btest-bg-run proxy-1 BROPATH=$BROPATH:.. CLUSTER_NODE=proxy-1 bro %INPUT -# @TEST-EXEC: sleep 1 +# @TEST-EXEC: sleep 2 # @TEST-EXEC: btest-bg-run worker-1 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff manager-1/notice.log @TEST-START-FILE cluster-layout.bro @@ -21,13 +21,44 @@ redef enum Notice::Type += { Test_Notice, }; +event remote_connection_closed(p: event_peer) + { + terminate(); + } + +global ready: event(); + +redef Cluster::manager2worker_events += /ready/; + event delayed_notice() { if ( Cluster::node == "worker-1" ) NOTICE([$note=Test_Notice, $msg="test notice!"]); } -event bro_init() +@if ( Cluster::local_node_type() == Cluster::WORKER ) + +event ready() { schedule 1secs { delayed_notice() }; } + +@endif + +@if ( Cluster::local_node_type() == Cluster::MANAGER ) + +global peer_count = 0; + +event remote_connection_handshake_done(p: event_peer) + { + peer_count = peer_count + 1; + if ( peer_count == 2 ) + event ready(); + } + +event Notice::log_notice(rec: Notice::Info) + { + terminate_communication(); + } + +@endif diff --git a/testing/btest/scripts/base/frameworks/notice/suppression-cluster.bro b/testing/btest/scripts/base/frameworks/notice/suppression-cluster.bro index b812c6451d..5010da82cc 100644 --- a/testing/btest/scripts/base/frameworks/notice/suppression-cluster.bro +++ b/testing/btest/scripts/base/frameworks/notice/suppression-cluster.bro @@ -2,10 +2,10 @@ # # @TEST-EXEC: btest-bg-run manager-1 BROPATH=$BROPATH:.. CLUSTER_NODE=manager-1 bro %INPUT # @TEST-EXEC: btest-bg-run proxy-1 BROPATH=$BROPATH:.. CLUSTER_NODE=proxy-1 bro %INPUT -# @TEST-EXEC: sleep 1 +# @TEST-EXEC: sleep 2 # @TEST-EXEC: btest-bg-run worker-1 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro %INPUT # @TEST-EXEC: btest-bg-run worker-2 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-2 bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 10 +# @TEST-EXEC: btest-bg-wait 20 # @TEST-EXEC: btest-diff manager-1/notice.log @TEST-START-FILE cluster-layout.bro @@ -23,6 +23,15 @@ redef enum Notice::Type += { Test_Notice, }; +event remote_connection_closed(p: event_peer) + { + terminate(); + } + +global ready: event(); + +redef Cluster::manager2worker_events += /ready/; + event delayed_notice() { NOTICE([$note=Test_Notice, @@ -30,10 +39,33 @@ event delayed_notice() $identifier="this identifier is static"]); } -event bro_init() &priority=5 - { +@if ( Cluster::local_node_type() == Cluster::WORKER ) + +event ready() + { if ( Cluster::node == "worker-1" ) schedule 4secs { delayed_notice() }; if ( Cluster::node == "worker-2" ) schedule 1secs { delayed_notice() }; + } + +event Notice::suppressed(n: Notice::Info) + { + if ( Cluster::node == "worker-1" ) + terminate_communication(); } + +@endif + +@if ( Cluster::local_node_type() == Cluster::MANAGER ) + +global peer_count = 0; + +event remote_connection_handshake_done(p: event_peer) + { + peer_count = peer_count + 1; + if ( peer_count == 3 ) + event ready(); + } + +@endif From 292bf61ae8cbdae6773b675ad5d33884c7fc7fd4 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 13 Sep 2012 12:59:40 -0500 Subject: [PATCH 4/7] Unit test reliability adjustment. Sometimes manager node was shutting everything down before others had a chance to generate output. It now waits for all nodes to fully connect with each other. --- .../base/frameworks/cluster/start-it-up.bro | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro b/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro index 89f8d6b168..acb9c3676a 100644 --- a/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro +++ b/testing/btest/scripts/base/frameworks/cluster/start-it-up.bro @@ -1,11 +1,13 @@ # @TEST-SERIALIZE: comm # # @TEST-EXEC: btest-bg-run manager-1 BROPATH=$BROPATH:.. CLUSTER_NODE=manager-1 bro %INPUT +# @TEST-EXEC: sleep 1 # @TEST-EXEC: btest-bg-run proxy-1 BROPATH=$BROPATH:.. CLUSTER_NODE=proxy-1 bro %INPUT # @TEST-EXEC: btest-bg-run proxy-2 BROPATH=$BROPATH:.. CLUSTER_NODE=proxy-2 bro %INPUT +# @TEST-EXEC: sleep 1 # @TEST-EXEC: btest-bg-run worker-1 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-1 bro %INPUT # @TEST-EXEC: btest-bg-run worker-2 BROPATH=$BROPATH:.. CLUSTER_NODE=worker-2 bro %INPUT -# @TEST-EXEC: btest-bg-wait 15 +# @TEST-EXEC: btest-bg-wait 30 # @TEST-EXEC: btest-diff manager-1/.stdout # @TEST-EXEC: btest-diff proxy-1/.stdout # @TEST-EXEC: btest-diff proxy-2/.stdout @@ -22,17 +24,39 @@ redef Cluster::nodes = { }; @TEST-END-FILE +global fully_connected: event(); + global peer_count = 0; +global fully_connected_nodes = 0; + +event fully_connected() + { + fully_connected_nodes = fully_connected_nodes + 1; + if ( Cluster::node == "manager-1" ) + { + if ( peer_count == 4 && fully_connected_nodes == 4 ) + terminate_communication(); + } + } + +redef Cluster::worker2manager_events += /fully_connected/; +redef Cluster::proxy2manager_events += /fully_connected/; + event remote_connection_handshake_done(p: event_peer) { print "Connected to a peer"; + peer_count = peer_count + 1; if ( Cluster::node == "manager-1" ) { - peer_count = peer_count + 1; - if ( peer_count == 4 ) + if ( peer_count == 4 && fully_connected_nodes == 4 ) terminate_communication(); } + else + { + if ( peer_count == 2 ) + event fully_connected(); + } } event remote_connection_closed(p: event_peer) From 6d1abdb661b98726c2c77e171bbab0a65e024f54 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 13 Sep 2012 16:47:40 -0500 Subject: [PATCH 5/7] Adjusting Mac binary packaging script. Setting CMAKE_PREFIX_PATH helps link against standard system libs instead of ones that come from other package manager (e.g. MacPorts). Changed to allow only more recent CMake versions to create packages due to poorer clang compiler support in older versions, important since clang is now the default compiler instead of gcc on Macs. --- pkg/make-mac-packages | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/make-mac-packages b/pkg/make-mac-packages index 829a64ca25..2930f8f393 100755 --- a/pkg/make-mac-packages +++ b/pkg/make-mac-packages @@ -3,7 +3,13 @@ # This script creates binary packages for Mac OS X. # They can be found in ../build/ after running. -./check-cmake || { exit 1; } +cmake -P /dev/stdin << "EOF" +if ( ${CMAKE_VERSION} VERSION_LESS 2.8.9 ) + message(FATAL_ERROR "CMake >= 2.8.9 required to build package") +endif () +EOF + +[ $? -ne 0 ] && exit 1; type sw_vers > /dev/null 2>&1 || { echo "Unable to get Mac OS X version" >&2; @@ -34,26 +40,26 @@ prefix=/opt/bro cd .. # Minimum Bro -CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ +CMAKE_PREFIX_PATH=/usr CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ --disable-broccoli --disable-broctl --pkg-name-prefix=Bro-minimal \ --binary-package ( cd build && make package ) # Full Bro package -CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ +CMAKE_PREFIX_PATH=/usr CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ --pkg-name-prefix=Bro --binary-package ( cd build && make package ) # Broccoli cd aux/broccoli -CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ +CMAKE_PREFIX_PATH=/usr CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ --binary-package ( cd build && make package && mv *.dmg ../../../build/ ) cd ../.. # Broctl cd aux/broctl -CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ +CMAKE_PREFIX_PATH=/usr CMAKE_OSX_ARCHITECTURES=${arch} ./configure --prefix=${prefix} \ --binary-package ( cd build && make package && mv *.dmg ../../../build/ ) cd ../.. From 6fbbf2829023036333231ffe00f89802b1f7bee0 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 14 Sep 2012 10:28:23 -0500 Subject: [PATCH 6/7] Update compile/dependency docs for OS X. --- doc/quickstart.rst | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/doc/quickstart.rst b/doc/quickstart.rst index cc18956836..68dc4561cb 100644 --- a/doc/quickstart.rst +++ b/doc/quickstart.rst @@ -1,5 +1,6 @@ .. _CMake: http://www.cmake.org .. _SWIG: http://www.swig.org +.. _Xcode: https://developer.apple.com/xcode/ .. _MacPorts: http://www.macports.org .. _Fink: http://www.finkproject.org .. _Homebrew: http://mxcl.github.com/homebrew @@ -85,17 +86,20 @@ The following dependencies are required to build Bro: * Mac OS X - Snow Leopard (10.6) comes with all required dependencies except for CMake_. + Compiling source code on Macs requires first downloading Xcode_, + then going through its "Preferences..." -> "Downloads" menus to + install the "Command Line Tools" component. - Lion (10.7) comes with all required dependencies except for CMake_ and SWIG_. + Lion (10.7) and Mountain Lion (10.7) come with all required + dependencies except for CMake_, SWIG_, and ``libmagic``. - Distributions of these dependencies can be obtained from the project websites - linked above, but they're also likely available from your preferred Mac OS X - package management system (e.g. MacPorts_, Fink_, or Homebrew_). + Distributions of these dependencies can be obtained from the project + websites linked above, but they're also likely available from your + preferred Mac OS X package management system (e.g. MacPorts_, Fink_, + or Homebrew_). - Note that the MacPorts ``swig`` package may not include any specific - language support so you may need to also install ``swig-ruby`` and - ``swig-python``. + Specifically for MacPorts, the ``swig``, ``swig-ruby``, ``swig-python`` + and ``file`` packages provide the required dependencies. Optional Dependencies ~~~~~~~~~~~~~~~~~~~~~ From 392b99b2fa4b7bdda267eca55d4cc57d85e88641 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 18 Sep 2012 16:52:12 -0500 Subject: [PATCH 7/7] Fix construction of ip6_ah (Authentication Header) record values. Authentication Headers with a Payload Len field set to zero would cause a crash due to invalid memory allocation because the previous code assumed Payload Len would always be great enough to contain all mandatory fields of the header. This changes it so the length of the header is explicitly checked before attempting to extract fields located past the minimum length (8 bytes) of an Authentication Header. Crashes due to this are only possible when handling script-layer events ipv6_ext_headers, new_packet, esp_packet, or teredo_*. Or also when implementing one of the discarder_check_* family of functions. Otherwise, Bro correctly parses past such a header. --- scripts/base/init-bare.bro | 8 ++++---- src/IP.cc | 11 ++++++++--- .../btest/Baseline/core.ipv6_zero_len_ah/output | 2 ++ testing/btest/Traces/ipv6_zero_len_ah.trace | Bin 0 -> 1320 bytes testing/btest/core/ipv6_zero_len_ah.test | 11 +++++++++++ 5 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 testing/btest/Baseline/core.ipv6_zero_len_ah/output create mode 100644 testing/btest/Traces/ipv6_zero_len_ah.trace create mode 100644 testing/btest/core/ipv6_zero_len_ah.test diff --git a/scripts/base/init-bare.bro b/scripts/base/init-bare.bro index ec75c76beb..cc3a40f54b 100644 --- a/scripts/base/init-bare.bro +++ b/scripts/base/init-bare.bro @@ -1135,10 +1135,10 @@ type ip6_ah: record { rsv: count; ## Security Parameter Index. spi: count; - ## Sequence number. - seq: count; - ## Authentication data. - data: string; + ## Sequence number, unset in the case that *len* field is zero. + seq: count &optional; + ## Authentication data, unset in the case that *len* field is zero. + data: string &optional; }; ## Values extracted from an IPv6 ESP extension header. diff --git a/src/IP.cc b/src/IP.cc index 45afd593a9..398aacf1ee 100644 --- a/src/IP.cc +++ b/src/IP.cc @@ -148,9 +148,14 @@ RecordVal* IPv6_Hdr::BuildRecordVal(VectorVal* chain) const rv->Assign(1, new Val(((ip6_ext*)data)->ip6e_len, TYPE_COUNT)); rv->Assign(2, new Val(ntohs(((uint16*)data)[1]), TYPE_COUNT)); rv->Assign(3, new Val(ntohl(((uint32*)data)[1]), TYPE_COUNT)); - rv->Assign(4, new Val(ntohl(((uint32*)data)[2]), TYPE_COUNT)); - uint16 off = 3 * sizeof(uint32); - rv->Assign(5, new StringVal(new BroString(data + off, Length() - off, 1))); + if ( Length() >= 12 ) + { + // Sequence Number and ICV fields can only be extracted if + // Payload Len was non-zero for this header. + rv->Assign(4, new Val(ntohl(((uint32*)data)[2]), TYPE_COUNT)); + uint16 off = 3 * sizeof(uint32); + rv->Assign(5, new StringVal(new BroString(data + off, Length() - off, 1))); + } } break; diff --git a/testing/btest/Baseline/core.ipv6_zero_len_ah/output b/testing/btest/Baseline/core.ipv6_zero_len_ah/output new file mode 100644 index 0000000000..d8db6a4c48 --- /dev/null +++ b/testing/btest/Baseline/core.ipv6_zero_len_ah/output @@ -0,0 +1,2 @@ +[orig_h=2000:1300::1, orig_p=128/icmp, resp_h=2000:1300::2, resp_p=129/icmp] +[ip=, ip6=[class=0, flow=0, len=166, nxt=51, hlim=255, src=2000:1300::1, dst=2000:1300::2, exts=[[id=51, hopopts=, dstopts=, routing=, fragment=, ah=[nxt=58, len=0, rsv=0, spi=0, seq=, data=], esp=, mobility=]]], tcp=, udp=, icmp=] diff --git a/testing/btest/Traces/ipv6_zero_len_ah.trace b/testing/btest/Traces/ipv6_zero_len_ah.trace new file mode 100644 index 0000000000000000000000000000000000000000..7c3922525c26f97d870d6c2c3aa0462e82315b4a GIT binary patch literal 1320 zcmca|c+)~A1{MYw`2U}Qff2~DHt-7wNoHd31F}JwgF$_t(qt|Mbs)R#ZUT^Gkg)o% zz#t4_!2lx~pQ(W%X{Sk8#RNw*05ZL?kclA-s1t;ZjX~Bz?0}lCfMGh*e$dHILq%IdTG28*V0EDslVVN<(c(4NM1c3&IU(bM){NMzjko*MnD-SRM zf-shlyoQ-7&_j}i@whn9k8BA*f?-&NjZp<6m0?K-6y`@?B-62kJf`VP=pm0Q4Lbni zb=oRI`S4!@D8j%g{Qo~7jb=JiJHr+^kUY9LBQzg^Y`G4!1#dn?&nZmkwstV6svp2& F3jp^*vyA`% literal 0 HcmV?d00001 diff --git a/testing/btest/core/ipv6_zero_len_ah.test b/testing/btest/core/ipv6_zero_len_ah.test new file mode 100644 index 0000000000..dc3acf8443 --- /dev/null +++ b/testing/btest/core/ipv6_zero_len_ah.test @@ -0,0 +1,11 @@ +# @TEST-EXEC: bro -r $TRACES/ipv6_zero_len_ah.trace %INPUT >output +# @TEST-EXEC: btest-diff output + +# Shouldn't crash, but we also won't have seq and data fields set of the ip6_ah +# record. + +event ipv6_ext_headers(c: connection, p: pkt_hdr) + { + print c$id; + print p; + }