diff --git a/NEWS b/NEWS index fbb9fc89a8..c713bcb14a 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,15 @@ Breaking Changes files. We tested builds of all of the existing third-party packages and only noticed one or two failures, but there is a possibility for breakage related to this cleanup. +- Network timestamps are not added to events by default anymore. Use the following + redef line to enable them: + + redef EventMetadata::add_network_timestamp = T; + + The background is that event metadata has become more generic and may incur + a small overhead when enabled. There's not enough users of network timestamp + metadata to justify the complexity of treating it separate. + New Functionality ----------------- @@ -65,6 +74,13 @@ Deprecated Functionality The replacement script doesn't populate the ``email_body_sections`` anymore either. +- The ``zeek::Event()`` constructor was deprecated. Use ``event_mgr::Enqueue()`` + or ``event_mgr::Dispatch()`` instead. + +- Passing ``ts`` as the last argument to ``EventMgr::Enqueue()`` has been deprecated + and will lead to compile time warnings. Use ``EventMgr::Enqueue(detail::MetadataVectorPtr meta, ...)`` + for populating ``meta`` accordingly. + Zeek 7.2.0 ========== diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index a6a3154f68..9fd71dae1a 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -592,6 +592,15 @@ export { id: EventMetadata::ID; ##< The registered :zeek:see:`EventMetadata::ID` value. val: any; ##< The value. Its type matches what was passed to :zeek:see:`EventMetadata::register`. }; + + ## Add network timestamp metadata to all events. + ## + ## Adding network timestamp metadata affects local and + ## remote events. Events scheduled have a network timestamp + ## of when the scheduled timer was supposed to expire, which + ## might be a value before the network_time() when the event + ## was actually dispatched. + const add_network_timestamp: bool = F &redef; } module FTP; diff --git a/src/Event.cc b/src/Event.cc index 7f61e062dd..7465fff410 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -12,6 +12,7 @@ #include "zeek/iosource/Manager.h" #include "zeek/plugin/Manager.h" +#include "const.bif.netvar_h" #include "event.bif.netvar_h" zeek::EventMgr zeek::event_mgr; @@ -52,6 +53,19 @@ Event::Event(const EventHandlerPtr& arg_handler, zeek::Args arg_args, util::deta Ref(obj); } +Event::Event(detail::EventMetadataVectorPtr arg_meta, const EventHandlerPtr& arg_handler, zeek::Args arg_args, + util::detail::SourceID arg_src, analyzer::ID arg_aid, Obj* arg_obj) + : handler(arg_handler), + args(std::move(arg_args)), + src(arg_src), + aid(arg_aid), + obj(arg_obj), + next_event(nullptr), + meta(std::move(arg_meta)) { + if ( obj ) + Ref(obj); +} + double Event::Time() const { if ( ! meta ) return 0.0; @@ -120,8 +134,60 @@ EventMgr::~EventMgr() { } void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID src, analyzer::ID aid, Obj* obj, - double ts) { - QueueEvent(new Event(h, std::move(vl), src, aid, obj, ts)); + DeprecatedTimestamp deprecated_ts) { + 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 + ts = run_state::network_time; + + // In v8.1 when the deprecated_ts parameters is gone: Just use run_state::network_time directly here. + meta = detail::MakeEventMetadataVector(ts); + } + else if ( ts >= 0.0 ) { + // EventMetadata::add_network_timestamp is false, but EventMgr::Enqueue() + // with an explicit (non-negative) timestamp is used. That's a deprecated + // API, but we continue to support it until v8.1. + meta = detail::MakeEventMetadataVector(ts); + } + + QueueEvent(new Event(std::move(meta), h, std::move(vl), src, aid, obj)); +} + +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 all events are supposed to have a network time attached, ensure + // that the meta vector was passed *and* contains a network timestamp. + bool has_time = false; + + if ( ! meta ) { + // No metadata vector at all, make one with a timestamp. + meta = detail::MakeEventMetadataVector(run_state::network_time); + } + else { + // Check all entries for a network timestamp + for ( const auto& m : *meta ) { + if ( m.Id() == static_cast(detail::MetadataType::NetworkTimestamp) ) { + has_time = true; + + if ( m.Val()->GetType()->Tag() != TYPE_TIME ) { + // This should've been caught during parsing. + zeek::reporter->InternalError("event metadata timestamp has wrong type: %s", + obj_desc_short(m.Val()->GetType().get()).c_str()); + } + } + } + + if ( ! has_time ) { + auto tv = zeek::make_intrusive(run_state::network_time); + meta->push_back({static_cast(detail::MetadataType::NetworkTimestamp), std::move(tv)}); + } + } + } + + QueueEvent(new Event(std::move(meta), h, std::move(vl), src, aid, obj)); } void EventMgr::QueueEvent(Event* event) { @@ -150,7 +216,13 @@ void EventMgr::Dispatch(Event* event, bool no_remote) { } void EventMgr::Dispatch(const EventHandlerPtr& h, zeek::Args vl) { - auto* ev = new Event(h, std::move(vl)); + detail::EventMetadataVectorPtr meta; + + // If all events should have network timestamps, create the vector holding one. + if ( BifConst::EventMetadata::add_network_timestamp ) + meta = detail::MakeEventMetadataVector(run_state::network_time); + + auto* ev = new Event(std::move(meta), h, std::move(vl), util::detail::SOURCE_LOCAL, 0, nullptr); // Technically this isn't queued, but still give plugins a chance to // intercept the event and cancel or modify it if really wanted. diff --git a/src/Event.h b/src/Event.h index 68c235d6ee..1edda36bc3 100644 --- a/src/Event.h +++ b/src/Event.h @@ -53,6 +53,7 @@ EventMetadataVectorPtr MakeEventMetadataVector(double t); class Event final : public Obj { public: + [[deprecated("Remove in v8.1: Do not instantiate raw events. Use EventMgr::Dispatch() or EventMgr::Enqueue().")]] Event(const EventHandlerPtr& handler, zeek::Args args, util::detail::SourceID src = util::detail::SOURCE_LOCAL, analyzer::ID aid = 0, Obj* obj = nullptr, double ts = run_state::network_time); @@ -70,6 +71,10 @@ public: private: friend class EventMgr; + // Construct an event with a metadata vector. Passing arg_meta as nullptr is explicitly allowed. + Event(detail::EventMetadataVectorPtr arg_meta, const EventHandlerPtr& arg_handler, zeek::Args arg_args, + util::detail::SourceID arg_src, analyzer::ID arg_aid, Obj* arg_obj); + // This method is protected to make sure that everybody goes through // EventMgr::Dispatch(). void Dispatch(bool no_remote = false); @@ -84,6 +89,8 @@ private: }; class EventMgr final : public Obj, public iosource::IOSource { + class DeprecatedTimestamp; + public: ~EventMgr() override; @@ -99,10 +106,10 @@ public: * @param obj an arbitrary object to use as a "cookie" or just hold a * reference to until dispatching the event. * @param ts timestamp at which the event is intended to be executed - * (defaults to current network time). + * (defaults to current network time - deprecated). */ void Enqueue(const EventHandlerPtr& h, zeek::Args vl, util::detail::SourceID src = util::detail::SOURCE_LOCAL, - analyzer::ID aid = 0, Obj* obj = nullptr, double ts = run_state::network_time); + analyzer::ID aid = 0, Obj* obj = nullptr, DeprecatedTimestamp ts = {}); /** * A version of Enqueue() taking a variable number of arguments. @@ -113,6 +120,19 @@ public: return Enqueue(h, zeek::Args{std::forward(args)...}); } + /** + * Enqueue() with metadata vector support. + * @param meta Metadata to attach to the event, can be nullptr. + * @param h reference to the event handler to later call. + * @param vl the argument list to the event handler call. + * @param src indicates the origin of the event (local versus remote). + * @param aid identifies the protocol analyzer generating the event. + * @param obj an arbitrary object to use as a "cookie" or just hold a + * reference to until dispatching the event. + */ + void Enqueue(detail::EventMetadataVectorPtr meta, const EventHandlerPtr& h, zeek::Args vl, + util::detail::SourceID src = util::detail::SOURCE_LOCAL, analyzer::ID aid = 0, Obj* obj = nullptr); + [[deprecated("Remove in v8.1: Use Dispatch(handler, args) instead.")]] void Dispatch(Event* event, bool no_remote = false); @@ -162,6 +182,24 @@ public: uint64_t num_events_dispatched = 0; private: + /** + * Helper class to produce a compile time warning if Enqueue() is called with an explicit timestamp. + * + * Remove in v8.1. + */ + class DeprecatedTimestamp { + public: + DeprecatedTimestamp() : d(-1.0) {} + [[deprecated("Use overload EventMgr::Enqueue(EventMetadataVectorPtr meta, ...) to pass timestamp metadata")]] + /*implicit*/ DeprecatedTimestamp(double d) + : d(d) {} + + explicit operator double() const { return d; } + + private: + double d; + }; + void QueueEvent(Event* event); Event* current = nullptr; diff --git a/src/Expr.cc b/src/Expr.cc index 53405ba198..cddc9d800a 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -24,6 +24,8 @@ #include "zeek/script_opt/Expr.h" #include "zeek/script_opt/ScriptOpt.h" +#include "const.bif.netvar_h" + namespace zeek::detail { const char* expr_name(ExprTag t) { @@ -3827,11 +3829,17 @@ ScheduleTimer::ScheduleTimer(const EventHandlerPtr& arg_event, Args arg_args, do void ScheduleTimer::Dispatch(double /* t */, bool /* is_expire */) { if ( event ) { - // An event's intended timestamp might be in the past as timer expiration is driven by - // network time. Guarantee that the intended timestamp is never in the future (e.g., - // when all timers are expired on shutdown). - auto ts = std::min(this->Time(), run_state::network_time); - event_mgr.Enqueue(event, std::move(args), util::detail::SOURCE_LOCAL, 0, nullptr, ts); + detail::EventMetadataVectorPtr meta; + + if ( BifConst::EventMetadata::add_network_timestamp ) { + // An event's intended timestamp might be in the past as timer expiration is driven by + // network time. Guarantee that the intended timestamp is never in the future (e.g., + // when all timers are expired on shutdown). + auto ts = std::min(this->Time(), run_state::network_time); + meta = detail::MakeEventMetadataVector(ts); + } + + event_mgr.Enqueue(std::move(meta), event, std::move(args)); } } diff --git a/src/const.bif b/src/const.bif index 822b0d30a6..482a325588 100644 --- a/src/const.bif +++ b/src/const.bif @@ -34,3 +34,5 @@ const Log::flush_interval: interval; const Log::write_buffer_size: count; const Storage::expire_interval: interval; + +const EventMetadata::add_network_timestamp: bool; diff --git a/testing/btest/broker/remote_event_auto_ts.zeek b/testing/btest/broker/remote_event_auto_ts.zeek index 49b26d846b..42050ae458 100644 --- a/testing/btest/broker/remote_event_auto_ts.zeek +++ b/testing/btest/broker/remote_event_auto_ts.zeek @@ -17,6 +17,7 @@ # @TEST-START-FILE send.zeek redef exit_only_after_terminate = T; +redef EventMetadata::add_network_timestamp = T; global runs = 0; global ping: event(msg: string, intended_ts: time); @@ -58,6 +59,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) # @TEST-START-FILE recv.zeek redef exit_only_after_terminate = T; +redef EventMetadata::add_network_timestamp = T; global msg_count = 0; diff --git a/testing/btest/broker/remote_event_schedule_ts.zeek b/testing/btest/broker/remote_event_schedule_ts.zeek index fe90f2b0cb..4b631ed95b 100644 --- a/testing/btest/broker/remote_event_schedule_ts.zeek +++ b/testing/btest/broker/remote_event_schedule_ts.zeek @@ -17,6 +17,7 @@ # @TEST-START-FILE send.zeek redef exit_only_after_terminate = T; +redef EventMetadata::add_network_timestamp = T; global runs = 0; global ping: event(msg: string, intended_ts: time, publish_ts: time); @@ -64,6 +65,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) # @TEST-START-FILE recv.zeek redef exit_only_after_terminate = T; +redef EventMetadata::add_network_timestamp = T; global msg_count = 0; diff --git a/testing/btest/broker/remote_event_ts.zeek b/testing/btest/broker/remote_event_ts.zeek index f822fde332..d9b8c2fadd 100644 --- a/testing/btest/broker/remote_event_ts.zeek +++ b/testing/btest/broker/remote_event_ts.zeek @@ -15,6 +15,7 @@ # @TEST-START-FILE send.zeek redef exit_only_after_terminate = T; +redef EventMetadata::add_network_timestamp = T; global runs = 0; global ping: event(msg: string, intended_ts: time); @@ -54,6 +55,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) # @TEST-START-FILE recv.zeek redef exit_only_after_terminate = T; +redef EventMetadata::add_network_timestamp = T; global msg_count = 0; diff --git a/testing/btest/broker/remote_event_ts_compat.zeek b/testing/btest/broker/remote_event_ts_compat.zeek index 7c457f9911..991e425759 100644 --- a/testing/btest/broker/remote_event_ts_compat.zeek +++ b/testing/btest/broker/remote_event_ts_compat.zeek @@ -17,6 +17,7 @@ redef exit_only_after_terminate = T; redef allow_network_time_forward = F; +redef EventMetadata::add_network_timestamp = T; event zeek_init() { diff --git a/testing/btest/broker/web-socket-events-metadata.zeek b/testing/btest/broker/web-socket-events-metadata.zeek index 34ab366061..5581e12a0d 100644 --- a/testing/btest/broker/web-socket-events-metadata.zeek +++ b/testing/btest/broker/web-socket-events-metadata.zeek @@ -16,6 +16,7 @@ redef allow_network_time_forward = F; redef exit_only_after_terminate = T; redef Broker::disable_ssl = T; +redef EventMetadata::add_network_timestamp = T; global event_count = 0; diff --git a/testing/btest/language/event-ts-scheduled.zeek b/testing/btest/language/event-ts-scheduled.zeek index bc0088845f..2a0ba07d55 100644 --- a/testing/btest/language/event-ts-scheduled.zeek +++ b/testing/btest/language/event-ts-scheduled.zeek @@ -1,6 +1,8 @@ # @TEST-EXEC: zeek -b -r $TRACES/ticks-dns-1hr.pcap %INPUT > out # @TEST-EXEC: btest-diff out +redef EventMetadata::add_network_timestamp = T; + global runs = 0; event test(schedule_time: time) diff --git a/testing/btest/language/event-ts.zeek b/testing/btest/language/event-ts.zeek index f35c3e5994..b4a8ff39ab 100644 --- a/testing/btest/language/event-ts.zeek +++ b/testing/btest/language/event-ts.zeek @@ -4,6 +4,8 @@ # Note: We use a PCAP with DNS queries only so that we have a single packet per # time step. Thus the run loop will be executed only once per time step. +redef EventMetadata::add_network_timestamp = T; + global runs = -1; event test(depth: count)