diff --git a/CHANGES b/CHANGES index c10d9f25c9..242dd9a114 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ + +3.3.0-dev.585 | 2020-12-01 14:42:54 +0000 + + * Retry loading plugins on failure to resolve to dependencies. + Closes #1179. (Robin Sommer, Corelight) + 3.3.0-dev.580 | 2020-11-30 14:07:39 -0700 * Find correct zeek namespace in debug logger macros. diff --git a/VERSION b/VERSION index b0315256e1..09e8a27d65 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.3.0-dev.580 +3.3.0-dev.585 diff --git a/src/plugin/Manager.cc b/src/plugin/Manager.cc index 6f20c7b516..614e100ca1 100644 --- a/src/plugin/Manager.cc +++ b/src/plugin/Manager.cc @@ -140,7 +140,7 @@ void Manager::SearchDynamicPlugins(const std::string& dir) closedir(d); } -bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_not_found) +bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_not_found, std::vector* errors) { dynamic_plugin_map::iterator m = dynamic_plugins.find(util::strtolower(name)); @@ -160,7 +160,7 @@ bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_ return true; } - reporter->Error("plugin %s is not available", name.c_str()); + errors->push_back(util::fmt("plugin %s is not available", name.c_str())); return false; } @@ -175,6 +175,72 @@ bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_ DBG_LOG(DBG_PLUGINS, "Activating plugin %s", name.c_str()); + // Load shared libraries. + + string dypattern = dir + "/lib/*." + HOST_ARCHITECTURE + DYNAMIC_PLUGIN_SUFFIX; + + DBG_LOG(DBG_PLUGINS, " Searching for shared libraries %s", dypattern.c_str()); + + glob_t gl; + + if ( glob(dypattern.c_str(), 0, 0, &gl) == 0 ) + { + for ( size_t i = 0; i < gl.gl_pathc; i++ ) + { + const char* path = gl.gl_pathv[i]; + + current_plugin = nullptr; + current_dir = dir.c_str(); + current_sopath = path; + void* hdl = dlopen(path, RTLD_NOW | RTLD_GLOBAL); + + if ( ! hdl ) + { + const char* err = dlerror(); + errors->push_back(util::fmt("cannot load plugin library %s: %s", path, err ? err : "")); + return false; + } + + if ( ! current_plugin ) { + errors->push_back(util::fmt("load plugin library %s did not instantiate a plugin", path)); + return false; + } + + current_plugin->SetDynamic(true); + current_plugin->DoConfigure(); + DBG_LOG(DBG_PLUGINS, " InitialzingComponents"); + current_plugin->InitializeComponents(); + + plugins_by_path.insert(std::make_pair(util::detail::normalize_path(dir), current_plugin)); + + // We execute the pre-script initialization here; this in + // fact could be *during* script initialization if we got + // triggered via @load-plugin. + current_plugin->InitPreScript(); + + // Make sure the name the plugin reports is consistent with + // what we expect from its magic file. + if ( util::strtolower(current_plugin->Name()) != util::strtolower(name) ) { + errors->push_back(util::fmt("inconsistent plugin name: %s vs %s", + current_plugin->Name().c_str(), name.c_str())); + return false; + } + + current_dir = nullptr; + current_sopath = nullptr; + current_plugin = nullptr; + + DBG_LOG(DBG_PLUGINS, " Loaded %s", path); + } + + globfree(&gl); + } + + else + { + DBG_LOG(DBG_PLUGINS, " No shared library found"); + } + // Add the "scripts" and "bif" directories to ZEEKPATH. std::string scripts = dir + "scripts"; @@ -227,104 +293,72 @@ bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_ } } - // Load shared libraries. - - string dypattern = dir + "/lib/*." + HOST_ARCHITECTURE + DYNAMIC_PLUGIN_SUFFIX; - - DBG_LOG(DBG_PLUGINS, " Searching for shared libraries %s", dypattern.c_str()); - - glob_t gl; - - if ( glob(dypattern.c_str(), 0, 0, &gl) == 0 ) - { - for ( size_t i = 0; i < gl.gl_pathc; i++ ) - { - const char* path = gl.gl_pathv[i]; - - current_plugin = nullptr; - current_dir = dir.c_str(); - current_sopath = path; - void* hdl = dlopen(path, RTLD_LAZY | RTLD_GLOBAL); - - if ( ! hdl ) - { - const char* err = dlerror(); - reporter->FatalError("cannot load plugin library %s: %s", path, err ? err : ""); - } - - if ( ! current_plugin ) - reporter->FatalError("load plugin library %s did not instantiate a plugin", path); - - current_plugin->SetDynamic(true); - current_plugin->DoConfigure(); - DBG_LOG(DBG_PLUGINS, " InitialzingComponents"); - current_plugin->InitializeComponents(); - - plugins_by_path.insert(std::make_pair(util::detail::normalize_path(dir), current_plugin)); - - // We execute the pre-script initialization here; this in - // fact could be *during* script initialization if we got - // triggered via @load-plugin. - current_plugin->InitPreScript(); - - // Make sure the name the plugin reports is consistent with - // what we expect from its magic file. - if ( util::strtolower(current_plugin->Name()) != util::strtolower(name) ) - reporter->FatalError("inconsistent plugin name: %s vs %s", - current_plugin->Name().c_str(), name.c_str()); - - current_dir = nullptr; - current_sopath = nullptr; - current_plugin = nullptr; - - DBG_LOG(DBG_PLUGINS, " Loaded %s", path); - } - - globfree(&gl); - } - - else - { - DBG_LOG(DBG_PLUGINS, " No shared library found"); - } - // Mark this plugin as activated by clearing the path. m->second.clear(); return true; } -bool Manager::ActivateDynamicPlugin(const std::string& name) +void Manager::ActivateDynamicPlugin(const std::string& name) { - if ( ! ActivateDynamicPluginInternal(name) ) - return false; - - UpdateInputFiles(); - return true; + std::vector errors; + if ( ActivateDynamicPluginInternal(name, false, &errors) ) + UpdateInputFiles(); + else + // Reschedule for another attempt later. + requested_plugins.insert(std::move(name)); } -bool Manager::ActivateDynamicPlugins(bool all) - { +void Manager::ActivateDynamicPlugins(bool all) { + // Tracks plugins we need to activate as pairs of their names and booleans + // indicating whether an activation failure is to be deemed a fatal error. + std::set> plugins_to_activate; + + // Activate plugins that were specifically requested. + for ( const auto& x : requested_plugins ) + plugins_to_activate.emplace(x, false); + // Activate plugins that our environment tells us to. vector p; util::tokenize_string(util::zeek_plugin_activate(), ",", &p); - for ( size_t n = 0; n < p.size(); ++n ) - ActivateDynamicPluginInternal(p[n], true); + for ( const auto& x : p ) + plugins_to_activate.emplace(x, true); if ( all ) { - for ( dynamic_plugin_map::const_iterator i = dynamic_plugins.begin(); - i != dynamic_plugins.end(); i++ ) + // Activate all other ones we discovered. + for ( const auto& x : dynamic_plugins ) + plugins_to_activate.emplace(x.first, false); + } + + // Now we keep iterating over all the plugins, trying to load them, for as + // long as we're successful for at least one further of them each round. + // Doing so ensures that we can resolve (non-cyclic) load dependencies + // independent of any particular order. + while ( ! plugins_to_activate.empty() ) { + std::vector errors; + auto plugins_left = plugins_to_activate; + + for ( const auto& x : plugins_to_activate ) { - if ( ! ActivateDynamicPluginInternal(i->first) ) - return false; + if ( ActivateDynamicPluginInternal(x.first, x.second, &errors) ) + plugins_left.erase(x); } + + if ( plugins_left.size() == plugins_to_activate.size() ) + { + // Could not load a single further plugin this round, that's fatal. + for ( const auto& msg : errors ) + reporter->Error("%s", msg.c_str()); + + reporter->FatalError("aborting after plugin errors"); + } + + plugins_to_activate = std::move(plugins_left); } UpdateInputFiles(); - - return true; } void Manager::UpdateInputFiles() diff --git a/src/plugin/Manager.h b/src/plugin/Manager.h index d34c5db07e..2fbb9570c7 100644 --- a/src/plugin/Manager.h +++ b/src/plugin/Manager.h @@ -2,9 +2,10 @@ #pragma once -#include #include +#include #include +#include #include "zeek/plugin/Plugin.h" #include "zeek/plugin/Component.h" @@ -79,28 +80,25 @@ public: * Activating a plugin involves loading its dynamic module, making its * bifs available, and adding its script paths to ZEEKPATH. * + * This attempts to activate the plugin immediately. If that fails for + * some reason, we schedule it to be retried later with + * ActivateDynamicPlugins(). + * * @param name The name of the plugin, as found previously by - * SearchPlugin(). - * - * @return True if the plugin has been loaded successfully. - * + ยท* SearchPlugin(). */ - bool ActivateDynamicPlugin(const std::string& name); + void ActivateDynamicPlugin(const std::string& name); /** - * Activates plugins that SearchDynamicPlugins() has previously discovered. - * The effect is the same all calling \a ActivePlugin(name) for each plugin. + * Activates plugins that SearchDynamicPlugins() has previously discovered, + * including any that have failed to load in prior calls to + * ActivateDynamicPlugin(). Aborts if any plugins fails to activate. * * @param all If true, activates all plugins that are found. If false, * activates only those that should always be activated unconditionally, - * as specified via the ZEEK_PLUGIN_ACTIVATE enviroment variable. In other - * words, it's \c true in standard mode and \c false in bare mode. - * - * @return True if all plugins have been loaded successfully. If one - * fails to load, the method stops there without loading any further ones - * and returns false. + * as specified via the ZEEK_PLUGIN_ACTIVATE environment variable. */ - bool ActivateDynamicPlugins(bool all); + void ActivateDynamicPlugins(bool all); /** * First-stage initializion of the manager. This is called early on @@ -413,11 +411,15 @@ public: static void RegisterBifFile(const char* plugin, bif_init_func c); private: - bool ActivateDynamicPluginInternal(const std::string& name, bool ok_if_not_found = false); + bool ActivateDynamicPluginInternal(const std::string& name, bool ok_if_not_found, std::vector* errors); void UpdateInputFiles(); void MetaHookPre(HookType hook, const HookArgumentList& args) const; void MetaHookPost(HookType hook, const HookArgumentList& args, HookArgument result) const; + // Plugins that were explicitly requested to be activated, but failed to + // load at first. + std::set requested_plugins; + // All found dynamic plugins, mapping their names to base directory. using dynamic_plugin_map = std::map; dynamic_plugin_map dynamic_plugins; diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index 2c0aece6bf..42c0abe3cb 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -604,17 +604,8 @@ SetupResult setup(int argc, char** argv, Options* zopts) file_mgr->InitPreScript(); zeekygen_mgr->InitPreScript(); - bool missing_plugin = false; - - for ( set::const_iterator i = requested_plugins.begin(); - i != requested_plugins.end(); i++ ) - { - if ( ! plugin_mgr->ActivateDynamicPlugin(*i) ) - missing_plugin = true; - } - - if ( missing_plugin ) - reporter->FatalError("Failed to activate requested dynamic plugin(s)."); + for ( const auto& x : requested_plugins ) + plugin_mgr->ActivateDynamicPlugin(std::move(x)); plugin_mgr->ActivateDynamicPlugins(! options.bare_mode); diff --git a/testing/btest/Baseline/plugins.plugin-load-dependency/output b/testing/btest/Baseline/plugins.plugin-load-dependency/output new file mode 100644 index 0000000000..e788232bd8 --- /dev/null +++ b/testing/btest/Baseline/plugins.plugin-load-dependency/output @@ -0,0 +1,18 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +Testing::Plugin2 - Plugin2 provides a load dependency for Plugin1 and Plugin3 (dynamic, version 1.0.0) +Testing::Plugin3 - Plugin3 has a load dependency on Plugin2 (dynamic, version 1.0.0) +in Plugin2 +in Plugin3 + +Testing::Plugin1 - Plugin1 has a load dependency on Plugin2 (dynamic, version 1.0.0) +Testing::Plugin2 - Plugin2 provides a load dependency for Plugin1 and Plugin3 (dynamic, version 1.0.0) +in Plugin1 +in Plugin2 + +Testing::Plugin1 - Plugin1 has a load dependency on Plugin2 (dynamic, version 1.0.0) +Testing::Plugin2 - Plugin2 provides a load dependency for Plugin1 and Plugin3 (dynamic, version 1.0.0) +Testing::Plugin3 - Plugin3 has a load dependency on Plugin2 (dynamic, version 1.0.0) +in Plugin1 +in Plugin2 +in Plugin2 +in Plugin3 diff --git a/testing/btest/plugins/bifs-and-scripts.sh b/testing/btest/plugins/bifs-and-scripts.sh index 911d279c11..345c1faa8f 100644 --- a/testing/btest/plugins/bifs-and-scripts.sh +++ b/testing/btest/plugins/bifs-and-scripts.sh @@ -51,7 +51,7 @@ EOF cat >src/foo.bif <("Hello from the plugin!"); %} event plugin_event%(foo: count%); diff --git a/testing/btest/plugins/plugin-load-dependency.zeek b/testing/btest/plugins/plugin-load-dependency.zeek new file mode 100644 index 0000000000..d9d78d7ebb --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency.zeek @@ -0,0 +1,31 @@ +# @TEST-EXEC: mkdir 1 +# @TEST-EXEC: cd 1 && ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Testing Plugin1 +# @TEST-EXEC: cp -r %DIR/plugin-load-dependency/1 . +# @TEST-EXEC: cd 1 && ./configure --zeek-dist=${DIST} && make + +# @TEST-EXEC: mkdir 2 +# @TEST-EXEC: cd 2 && ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Testing Plugin2 +# @TEST-EXEC: cp -r %DIR/plugin-load-dependency/2 . +# @TEST-EXEC: cd 2 && ./configure --zeek-dist=${DIST} && make + +# @TEST-EXEC: mkdir 3 +# @TEST-EXEC: cd 3 && ${DIST}/auxil/zeek-aux/plugin-support/init-plugin -u . Testing Plugin3 +# @TEST-EXEC: cp -r %DIR/plugin-load-dependency/3 . +# @TEST-EXEC: cd 3 && ./configure --zeek-dist=${DIST} && make + +# The following run will only work if Zeek loads plugin2 before plugin3 (which +# by alphabetical loading will be the case) +# @TEST-EXEC: ZEEK_PLUGIN_PATH=. zeek -b -N Testing::Plugin3 Testing::Plugin2 | grep -v Zeek:: | sort >> output +# +# @TEST-EXEC: echo >>output +# +# The following run will only work if Zeek loads plugin2 before plugin1 (which +# by alphabetical loading will not be the case). +# @TEST-EXEC: ZEEK_PLUGIN_PATH=. zeek -b -N Testing::Plugin1 Testing::Plugin2 | grep -v Zeek:: | sort >> output +# +# @TEST-EXEC: echo >>output +# +# Finally, try it with self-discovery of all three plugins too. +# @TEST-EXEC: ZEEK_PLUGIN_PATH=. zeek -N | grep -v Zeek:: | sort >> output +# +# @TEST-EXEC: btest-diff output diff --git a/testing/btest/plugins/plugin-load-dependency/.btest-ignore b/testing/btest/plugins/plugin-load-dependency/.btest-ignore new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/plugins/plugin-load-dependency/1/src/Plugin.cc b/testing/btest/plugins/plugin-load-dependency/1/src/Plugin.cc new file mode 100644 index 0000000000..76501c4bc9 --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency/1/src/Plugin.cc @@ -0,0 +1,23 @@ + +#include "Plugin.h" + +namespace btest::plugin::Testing_Plugin1 { Plugin plugin; } + +using namespace btest::plugin::Testing_Plugin1; + +extern void Plugin2_foo(); + +zeek::plugin::Configuration Plugin::Configure() + { + zeek::plugin::Configuration config; + config.name = "Testing::Plugin1"; + config.description = "Plugin1 has a load dependency on Plugin2"; + config.version.major = 1; + config.version.minor = 0; + config.version.patch = 0; + + printf("in Plugin1\n"); + Plugin2_foo(); + + return config; + } diff --git a/testing/btest/plugins/plugin-load-dependency/1/src/Plugin.h b/testing/btest/plugins/plugin-load-dependency/1/src/Plugin.h new file mode 100644 index 0000000000..18ccb8d319 --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency/1/src/Plugin.h @@ -0,0 +1,17 @@ + +#pragma once + +#include + +namespace btest::plugin::Testing_Plugin1 { + +class Plugin : public zeek::plugin::Plugin +{ +protected: + // Overridden from zeek::plugin::Plugin. + zeek::plugin::Configuration Configure() override; +}; + +extern Plugin plugin; + +} diff --git a/testing/btest/plugins/plugin-load-dependency/2/src/Plugin.cc b/testing/btest/plugins/plugin-load-dependency/2/src/Plugin.cc new file mode 100644 index 0000000000..fd6a28155e --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency/2/src/Plugin.cc @@ -0,0 +1,21 @@ + +#include "Plugin.h" + +namespace btest::plugin::Testing_Plugin2 { Plugin plugin; } + +using namespace btest::plugin::Testing_Plugin2; + +void Plugin2_foo() { + printf("in Plugin2\n"); +} + +zeek::plugin::Configuration Plugin::Configure() + { + zeek::plugin::Configuration config; + config.name = "Testing::Plugin2"; + config.description = "Plugin2 provides a load dependency for Plugin1 and Plugin3"; + config.version.major = 1; + config.version.minor = 0; + config.version.patch = 0; + return config; + } diff --git a/testing/btest/plugins/plugin-load-dependency/2/src/Plugin.h b/testing/btest/plugins/plugin-load-dependency/2/src/Plugin.h new file mode 100644 index 0000000000..8e9c69aecb --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency/2/src/Plugin.h @@ -0,0 +1,17 @@ + +#pragma once + +#include + +namespace btest::plugin::Testing_Plugin2 { + +class Plugin : public zeek::plugin::Plugin +{ +protected: + // Overridden from zeek::plugin::Plugin. + zeek::plugin::Configuration Configure() override; +}; + +extern Plugin plugin; + +} diff --git a/testing/btest/plugins/plugin-load-dependency/3/src/Plugin.cc b/testing/btest/plugins/plugin-load-dependency/3/src/Plugin.cc new file mode 100644 index 0000000000..68d878ad55 --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency/3/src/Plugin.cc @@ -0,0 +1,23 @@ + +#include "Plugin.h" + +namespace btest::plugin::Testing_Plugin3 { Plugin plugin; } + +using namespace btest::plugin::Testing_Plugin3; + +extern void Plugin2_foo(); + +zeek::plugin::Configuration Plugin::Configure() + { + zeek::plugin::Configuration config; + config.name = "Testing::Plugin3"; + config.description = "Plugin3 has a load dependency on Plugin2"; + config.version.major = 1; + config.version.minor = 0; + config.version.patch = 0; + + printf("in Plugin3\n"); + Plugin2_foo(); + + return config; + } diff --git a/testing/btest/plugins/plugin-load-dependency/3/src/Plugin.h b/testing/btest/plugins/plugin-load-dependency/3/src/Plugin.h new file mode 100644 index 0000000000..b6b692f877 --- /dev/null +++ b/testing/btest/plugins/plugin-load-dependency/3/src/Plugin.h @@ -0,0 +1,17 @@ + +#pragma once + +#include + +namespace btest::plugin::Testing_Plugin3 { + +class Plugin : public zeek::plugin::Plugin +{ +protected: + // Overridden from zeek::plugin::Plugin. + zeek::plugin::Configuration Configure() override; +}; + +extern Plugin plugin; + +}