diff --git a/CHANGES b/CHANGES index e4e7b4ef2e..123fd7e368 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,19 @@ +8.0.0-dev.209 | 2025-05-26 16:08:44 +0200 + + * btest: Add test for Cluster::hello zero-timestamp (Arne Welzel, Corelight) + + * EventMgr/Enqueue: Add automatic timestamp metadata to local events, only (Arne Welzel, Corelight) + + It seems less surprising if only local events receive automatic network + timestamp metadata. For remote events the automatic value will most + likely be misleading. + + * cluster and broker: Propagate zero-timestamp as metadata, too. (Arne Welzel, Corelight) + + This will be cleaned up later to just pass all contained metadata from + a cluster event to the queued event, but for now do this here, otherwise + we break some internal tests. + 8.0.0-dev.204 | 2025-05-23 12:13:41 -0700 * Redis: bump version of hiredis required (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index 1f14a05df7..8b5d41b527 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.0.0-dev.204 +8.0.0-dev.209 diff --git a/src/Event.cc b/src/Event.cc index c6af18be0d..8164ba3dd8 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -165,8 +165,13 @@ void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID detail::EventMetadataVectorPtr meta; double ts = double(deprecated_ts); - if ( BifConst::EventMetadata::add_network_timestamp ) { - if ( ts < 0.0 ) // default -1.0, modify to current network_time + if ( src == util::detail::SOURCE_LOCAL && BifConst::EventMetadata::add_network_timestamp ) { + // If this is a local event and EventMetadata::add_network_timestamp is + // enabled automatically set the network timestamp for this event to the + // current network time when it is < 0 (default is -1.0). + // + // See the other Enqueue() implementation for the local motivation. + if ( ts < 0.0 ) ts = run_state::network_time; // In v8.1 when the deprecated_ts parameters is gone: Just use run_state::network_time directly here. @@ -184,9 +189,15 @@ void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID void EventMgr::Enqueue(detail::EventMetadataVectorPtr meta, const EventHandlerPtr& h, Args vl, util::detail::SourceID src, analyzer::ID aid, Obj* obj) { - if ( BifConst::EventMetadata::add_network_timestamp ) { + if ( src == util::detail::SOURCE_LOCAL && BifConst::EventMetadata::add_network_timestamp ) { // If all events are supposed to have a network time attached, ensure // that the meta vector was passed *and* contains a network timestamp. + // + // This is only done for local events, however. For remote events (src == BROKER) + // that do not hold network timestamp metadata, it seems less surprising to keep + // it unset. If it is required that a remote node sends *their* network timestamp, + // defaulting to this node's network time seems more confusing and error prone + // than just leaving it unset and having the consumer deal with the situation. bool has_time = false; if ( ! meta ) { diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index e42bed29ab..2e36509a2f 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -1657,7 +1657,7 @@ void Manager::ProcessMessage(std::string_view topic, broker::zeek::Event& ev) { if ( vl.size() == args.size() ) { zeek::detail::EventMetadataVectorPtr meta; - if ( ts > 0.0 ) + if ( ts >= 0.0 ) meta = zeek::detail::MakeEventMetadataVector(ts); event_mgr.Enqueue(std::move(meta), handler, std::move(vl), util::detail::SOURCE_BROKER); diff --git a/src/cluster/Backend.cc b/src/cluster/Backend.cc index f7300e22d6..ae23a5fbf1 100644 --- a/src/cluster/Backend.cc +++ b/src/cluster/Backend.cc @@ -24,7 +24,7 @@ using namespace zeek::cluster; bool detail::LocalEventHandlingStrategy::DoProcessEvent(std::string_view topic, detail::Event e) { zeek::detail::EventMetadataVectorPtr meta; - if ( auto ts = e.Timestamp(); ts > 0.0 ) + if ( auto ts = e.Timestamp(); ts >= 0.0 ) meta = zeek::detail::MakeEventMetadataVector(e.Timestamp()); zeek::event_mgr.Enqueue(std::move(meta), e.Handler(), std::move(e.Args()), util::detail::SOURCE_BROKER); diff --git a/testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..manager..stdout b/testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..manager..stdout new file mode 100644 index 0000000000..d07af135ca --- /dev/null +++ b/testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..manager..stdout @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +Cluster::hello name=worker-1 is_remote_event=T metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=0.0]] +Cluster::node_up name=worker-1 is_remote_event=F metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=XXXXXXXXXX.XXXXXX]] diff --git a/testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..worker-1..stdout b/testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..worker-1..stdout new file mode 100644 index 0000000000..09db3fb744 --- /dev/null +++ b/testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..worker-1..stdout @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +Cluster::hello name=manager is_remote_event=T metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=XXXXXXXXXX.XXXXXX]] +Cluster::node_up name=manager is_remote_event=F metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=0.0]] diff --git a/testing/btest/cluster/broker/cluster-hello-zero-network-timestamp.zeek b/testing/btest/cluster/broker/cluster-hello-zero-network-timestamp.zeek new file mode 100644 index 0000000000..49541794ae --- /dev/null +++ b/testing/btest/cluster/broker/cluster-hello-zero-network-timestamp.zeek @@ -0,0 +1,48 @@ +# @TEST-DOC: Ensure Cluster::hello sent by a worker with a zero network time is observed with a zero network timestamp by the manager. +# +# @TEST-PORT: BROKER_MANAGER_PORT +# @TEST-PORT: BROKER_WORKER1_PORT +# +# @TEST-EXEC: cp $FILES/broker/cluster-layout.zeek . +# +# @TEST-EXEC: zeek -b --parse-only %INPUT +# +# @TEST-EXEC: btest-bg-run manager "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=manager zeek -b %INPUT" +# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=worker-1 zeek -b %INPUT" +# +# @TEST-EXEC: btest-bg-wait 30 +# @TEST-EXEC: btest-diff ./manager/.stdout +# @TEST-EXEC: btest-diff ./worker-1/.stdout + +redef allow_network_time_forward = F; +redef EventMetadata::add_network_timestamp = T; + +event do_terminate() + { + terminate(); + } + +event zeek_init() + { + # Set the manager's time to non-zero, the worker continues to be at 0.0. + if ( Cluster::local_node_type() == Cluster::MANAGER ) + set_network_time(double_to_time(1748256346)); + } + +event Cluster::hello(name: string, id: string) + { + print fmt("Cluster::hello name=%s is_remote_event=%s metadata=%s", name, is_remote_event(), EventMetadata::current_all()); + } + +event Cluster::node_up(name: string, id: string) + { + print fmt("Cluster::node_up name=%s is_remote_event=%s metadata=%s", name, is_remote_event(), EventMetadata::current_all()); + + if ( Cluster::local_node_type() == Cluster::MANAGER ) + Cluster::publish(Cluster::worker_topic, do_terminate); + } + +event Cluster::node_down(name: string, id: string) + { + terminate(); + }