From 3ec9fb0f7f9485e87803b51ff0554e7be842487f Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Tue, 25 Jun 2019 13:29:41 -0700 Subject: [PATCH] Change notices to be processed on worker. In the past they were processed on the manager - which requires big records to be sent around. This has a potential of incompatibilities if someone relied on global state for notice processing. GH-214 --- NEWS | 5 + .../frameworks/notice/actions/pp-alarms.zeek | 76 ++++++------- scripts/base/frameworks/notice/main.zeek | 100 +++++++++--------- .../base/frameworks/notice/cluster.zeek | 8 +- 4 files changed, 98 insertions(+), 91 deletions(-) diff --git a/NEWS b/NEWS index 477a532f91..5b03a8d51b 100644 --- a/NEWS +++ b/NEWS @@ -338,6 +338,11 @@ Changed Functionality passed to any other functions for further processing. The remainder of the ``ocsp_response_bytes`` is unchanged. +- For performance reasons, procesing on notices is now always performed on + the node on which the notice is raised. This is in difference to earlier + versions of Zeek, in which notices always were first sent to the manager + and processed there. + Removed Functionality --------------------- diff --git a/scripts/base/frameworks/notice/actions/pp-alarms.zeek b/scripts/base/frameworks/notice/actions/pp-alarms.zeek index ddfc45af6e..a5e058a89f 100644 --- a/scripts/base/frameworks/notice/actions/pp-alarms.zeek +++ b/scripts/base/frameworks/notice/actions/pp-alarms.zeek @@ -10,7 +10,7 @@ module Notice; export { ## Activate pretty-printed alarm summaries. const pretty_print_alarms = T &redef; - + ## Address to send the pretty-printed reports to. Default if not set is ## :zeek:id:`Notice::mail_dest`. ## @@ -20,10 +20,10 @@ export { ## the entry with an additional quote symbol (i.e., ">"). Many MUAs ## then highlight such lines differently. global flag_nets: set[subnet] &redef; - + ## Function that renders a single alarm. Can be overridden. global pretty_print_alarm: function(out: file, n: Info) &redef; - + ## Force generating mail file, even if reading from traces or no mail ## destination is defined. This is mainly for testing. global force_email_summaries = F &redef; @@ -39,7 +39,7 @@ function want_pp() : bool { if ( force_email_summaries ) return T; - + return (pretty_print_alarms && ! reading_traces() && (mail_dest != "" || mail_dest_pretty_printed != "")); } @@ -49,7 +49,7 @@ function pp_open() { if ( pp_alarms_open ) return; - + pp_alarms_open = T; pp_alarms = open(pp_alarms_name); } @@ -59,29 +59,29 @@ function pp_send(rinfo: Log::RotationInfo) { if ( ! pp_alarms_open ) return; - + write_file(pp_alarms, "\n\n--\n[Automatically generated]\n\n"); close(pp_alarms); pp_alarms_open = F; - + local from = strftime("%H:%M:%S", rinfo$open); local to = strftime("%H:%M:%S", rinfo$close); local subject = fmt("Alarm summary from %s-%s", from, to); local dest = mail_dest_pretty_printed != "" ? mail_dest_pretty_printed : mail_dest; - + if ( dest == "" ) # No mail destination configured, just leave the file alone. This is mainly for # testing. return; - + local headers = email_headers(subject, dest); - + local header_name = pp_alarms_name + ".tmp"; local header = open(header_name); write_file(header, headers + "\n"); close(header); - + system(fmt("/bin/cat %s %s | %s -t -oi && /bin/rm -f %s %s", header_name, pp_alarms_name, sendmail, header_name, pp_alarms_name)); } @@ -91,7 +91,7 @@ function pp_postprocessor(info: Log::RotationInfo): bool { if ( want_pp() ) pp_send(info); - + return T; } @@ -99,7 +99,7 @@ event zeek_init() { if ( ! want_pp() ) return; - + # This replaces the standard non-pretty-printing filter. Log::add_filter(Notice::ALARM_LOG, [$name="alarm-mail", $writer=Log::WRITER_NONE, @@ -111,13 +111,13 @@ hook notice(n: Notice::Info) &priority=-5 { if ( ! want_pp() ) return; - + if ( ACTION_ALARM !in n$actions ) return; - + if ( ! pp_alarms_open ) pp_open(); - + pretty_print_alarm(pp_alarms, n); } @@ -128,17 +128,17 @@ function do_msg(out: file, n: Info, line1: string, line2: string, line3: string, if ( n?$remote_location && n$remote_location?$country_code ) country = fmt(" (remote location %s)", n$remote_location$country_code); @endif - + line1 = cat(line1, country); - + local resolved = ""; - + if ( host1 != 0.0.0.0 ) resolved = fmt("%s # %s = %s", resolved, host1, name1); - + if ( host2 != 0.0.0.0 ) resolved = fmt("%s %s = %s", resolved, host2, name2); - + print out, line1; print out, line2; if ( line3 != "" ) @@ -152,7 +152,7 @@ function do_msg(out: file, n: Info, line1: string, line2: string, line3: string, function pretty_print_alarm(out: file, n: Info) { local pdescr = ""; - + @if ( Cluster::is_enabled() ) pdescr = "local"; @@ -163,16 +163,16 @@ function pretty_print_alarm(out: file, n: Info) pdescr = fmt("<%s> ", pdescr); @endif - + local msg = fmt( "%s%s", pdescr, n$msg); - + local who = ""; local h1 = 0.0.0.0; local h2 = 0.0.0.0; - + local orig_p = ""; local resp_p = ""; - + if ( n?$id ) { h1 = n$id$orig_h; @@ -190,56 +190,56 @@ function pretty_print_alarm(out: file, n: Info) h1 = n$src; who = fmt("%s%s", h1, (n?$p ? fmt(":%s", n$p) : "")); } - + if ( n?$uid ) who = fmt("%s (uid %s)", who, n$uid ); - + local flag = (h1 in flag_nets || h2 in flag_nets); - + local line1 = fmt(">%s %D %s %s", (flag ? ">" : " "), network_time(), n$note, who); local line2 = fmt(" %s", msg); local line3 = n?$sub ? fmt(" %s", n$sub) : ""; - + if ( h1 == 0.0.0.0 ) { do_msg(out, n, line1, line2, line3, h1, "", h2, ""); return; } - + if ( reading_traces() ) { do_msg(out, n, line1, line2, line3, h1, "", h2, ""); return; } - + when ( local h1name = lookup_addr(h1) ) { - if ( h2 == 0.0.0.0 ) + if ( h2 == 0.0.0.0 ) { do_msg(out, n, line1, line2, line3, h1, h1name, h2, ""); return; } - + when ( local h2name = lookup_addr(h2) ) { do_msg(out, n, line1, line2, line3, h1, h1name, h2, h2name); return; } - timeout 5secs + timeout 5secs { do_msg(out, n, line1, line2, line3, h1, h1name, h2, "(dns timeout)"); return; } } - + timeout 5secs { - if ( h2 == 0.0.0.0 ) + if ( h2 == 0.0.0.0 ) { do_msg(out, n, line1, line2, line3, h1, "(dns timeout)", h2, ""); return; } - + when ( local h2name_ = lookup_addr(h2) ) { do_msg(out, n, line1, line2, line3, h1, "(dns timeout)", h2, h2name_); diff --git a/scripts/base/frameworks/notice/main.zeek b/scripts/base/frameworks/notice/main.zeek index ab4288de1a..9e43a9ed50 100644 --- a/scripts/base/frameworks/notice/main.zeek +++ b/scripts/base/frameworks/notice/main.zeek @@ -254,7 +254,7 @@ export { global log_mailing_postprocessor: function(info: Log::RotationInfo): bool; ## This is the event that is called as the entry point to the - ## notice framework by the global :zeek:id:`NOTICE` function. By the + ## notice framework by the global :zeek:id:`NOTICE` function. By the ## time this event is generated, default values have already been ## filled out in the :zeek:type:`Notice::Info` record and the notice ## policy has also been applied. @@ -273,6 +273,18 @@ export { ## identifier: The identifier string of the notice that should be suppressed. global begin_suppression: event(ts: time, suppress_for: interval, note: Type, identifier: string); + ## This is an internal event that is used to broadcast the begin_suppression + ## event over a cluster. + ## + ## ts: time indicating then when the notice to be suppressed occured. + ## + ## suppress_for: length of time that this notice should be suppressed. + ## + ## note: The :zeek:type:`Notice::Type` of the notice. + ## + ## identifier: The identifier string of the notice that should be suppressed. + global manager_begin_suppression: event(ts: time, suppress_for: interval, note: Type, identifier: string); + ## A function to determine if an event is supposed to be suppressed. ## ## n: The record containing the notice in question. @@ -314,17 +326,8 @@ export { ## rec: The record containing notice data before it is logged. global log_notice: event(rec: Info); - ## This is an internal wrapper for the global :zeek:id:`NOTICE` - ## function; disregard. - ## - ## n: The record of notice data. - global internal_NOTICE: function(n: Notice::Info); - - ## This is the event used to transport notices on the cluster. - ## - ## n: The notice information to be sent to the cluster manager for - ## further processing. - global cluster_notice: event(n: Notice::Info); + ## This is an internal function to populate policy records. + global apply_policy: function(n: Notice::Info); } module GLOBAL; @@ -334,17 +337,11 @@ function NOTICE(n: Notice::Info) if ( Notice::is_being_suppressed(n) ) return; - @if ( Cluster::is_enabled() ) - if ( Cluster::local_node_type() == Cluster::MANAGER ) - Notice::internal_NOTICE(n); - else - { - n$peer_name = n$peer_descr = Cluster::node; - Broker::publish(Cluster::manager_topic, Notice::cluster_notice, n); - } - @else - Notice::internal_NOTICE(n); - @endif + # Fill out fields that might be empty and do the policy processing. + Notice::apply_policy(n); + + # Generate the notice event with the notice. + hook Notice::notice(n); } module Notice; @@ -519,9 +516,12 @@ hook Notice::notice(n: Notice::Info) &priority=-5 if ( n?$identifier && [n$note, n$identifier] !in suppressing && n$suppress_for != 0secs ) - { + { event Notice::begin_suppression(n$ts, n$suppress_for, n$note, n$identifier); - } +@if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) + event Notice::manager_begin_suppression(n$ts, n$suppress_for, n$note, n$identifier); +@endif + } } event Notice::begin_suppression(ts: time, suppress_for: interval, note: Type, @@ -531,15 +531,27 @@ event Notice::begin_suppression(ts: time, suppress_for: interval, note: Type, suppressing[note, identifier] = suppress_until; } +@if ( Cluster::is_enabled() && Cluster::local_node_type() == Cluster::MANAGER ) event zeek_init() { - if ( ! Cluster::is_enabled() ) - return; - Broker::auto_publish(Cluster::worker_topic, Notice::begin_suppression); Broker::auto_publish(Cluster::proxy_topic, Notice::begin_suppression); } +event Notice::manager_begin_suppression(ts: time, suppress_for: interval, note: Type, + identifier: string) + { + event Notice::begin_suppression(ts, suppress_for, note, identifier); + } +@endif + +@if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) +event zeek_init() + { + Broker::auto_publish(Cluster::manager_topic, Notice::manager_begin_suppression); + } +@endif + function is_being_suppressed(n: Notice::Info): bool { if ( n?$identifier && [n$note, n$identifier] in suppressing ) @@ -605,6 +617,14 @@ function apply_policy(n: Notice::Info) if ( ! n?$ts ) n$ts = network_time(); +@if ( Cluster::is_enabled() ) + if ( ! n?$peer_name ) + n$peer_name = Cluster::node; + + if ( ! n?$peer_descr ) + n$peer_descr = Cluster::node; +@endif + if ( n?$f ) populate_file_info(n$f, n); @@ -652,28 +672,4 @@ function apply_policy(n: Notice::Info) # suppression interval given yet, the default is applied. if ( ! n?$suppress_for ) n$suppress_for = default_suppression_interval; - - # Delete the connection and file records if they're there so we - # aren't sending that to remote machines. It can cause problems - # due to the size of those records. - if ( n?$conn ) - delete n$conn; - if ( n?$iconn ) - delete n$iconn; - if ( n?$f ) - delete n$f; - } - -function internal_NOTICE(n: Notice::Info) - { - # Fill out fields that might be empty and do the policy processing. - apply_policy(n); - - # Generate the notice event with the notice. - hook Notice::notice(n); - } - -event Notice::cluster_notice(n: Notice::Info) - { - NOTICE(n); } diff --git a/testing/btest/scripts/base/frameworks/notice/cluster.zeek b/testing/btest/scripts/base/frameworks/notice/cluster.zeek index 06160f0309..5a8a5fdf4f 100644 --- a/testing/btest/scripts/base/frameworks/notice/cluster.zeek +++ b/testing/btest/scripts/base/frameworks/notice/cluster.zeek @@ -33,9 +33,15 @@ event delayed_notice() NOTICE([$note=Test_Notice, $msg="test notice!"]); } +event terminate_me() + { + terminate(); + } + event ready() { schedule 1secs { delayed_notice() }; + schedule 2secs { terminate_me() }; } @if ( Cluster::local_node_type() == Cluster::MANAGER ) @@ -50,7 +56,7 @@ event Cluster::node_up(name: string, id: string) Broker::publish(Cluster::worker_topic, ready); } -event Notice::log_notice(rec: Notice::Info) +event Cluster::node_down(name: string, id: string) { terminate(); }