From 064b39f2be358e83db5ed636b8631d5e6c74dcb0 Mon Sep 17 00:00:00 2001 From: Mark Stemm Date: Mon, 28 Nov 2016 14:39:17 -0800 Subject: [PATCH] Validate rule outputs when loading rules. Validate rule outputs when loading rules by attempting to create a formatter based on the rule's output field. If there's an error, it will propagate up through load_rules and cause falco to exit rather than discover the problem only when trying to format the event and the rule's output field. This required moving formats.{cpp,h} into the falco engine directory from the falco general directory. Note that these functions are loaded twice in the two lua states used by falco (engine and outputs). There's also a couple of minor cleanups: - falco_formats had a private instance variable that was unused, remove it. - rename the package for the falco_formats functions to formats instead of falco so it's more standalone. - don't throw a c++ exception in falco_formats::formatter. Instead generate a lua error, which is handled more cleanly. - free_formatter doesn't return any values, so set the return value of the function to 0. --- userspace/engine/CMakeLists.txt | 2 +- userspace/engine/falco_engine.cpp | 11 +++++++++++ userspace/{falco => engine}/formats.cpp | 8 ++++---- userspace/{falco => engine}/formats.h | 3 --- userspace/engine/lua/rule_loader.lua | 6 ++++++ userspace/falco/CMakeLists.txt | 2 +- userspace/falco/falco_outputs.cpp | 3 +++ userspace/falco/lua/output.lua | 6 +++--- 8 files changed, 29 insertions(+), 12 deletions(-) rename userspace/{falco => engine}/formats.cpp (94%) rename userspace/{falco => engine}/formats.h (97%) diff --git a/userspace/engine/CMakeLists.txt b/userspace/engine/CMakeLists.txt index dfc85495..96ec10a7 100644 --- a/userspace/engine/CMakeLists.txt +++ b/userspace/engine/CMakeLists.txt @@ -4,7 +4,7 @@ include_directories("${PROJECT_SOURCE_DIR}/../sysdig/userspace/libsinsp") include_directories("${PROJECT_BINARY_DIR}/userspace/engine") include_directories("${LUAJIT_INCLUDE}") -add_library(falco_engine STATIC rules.cpp falco_common.cpp falco_engine.cpp) +add_library(falco_engine STATIC rules.cpp falco_common.cpp falco_engine.cpp formats.cpp) target_include_directories(falco_engine PUBLIC "${LUAJIT_INCLUDE}") diff --git a/userspace/engine/falco_engine.cpp b/userspace/engine/falco_engine.cpp index 910a73b9..4fabf681 100644 --- a/userspace/engine/falco_engine.cpp +++ b/userspace/engine/falco_engine.cpp @@ -24,6 +24,8 @@ along with falco. If not, see . #include "falco_engine.h" #include "config_falco_engine.h" +#include "formats.h" + extern "C" { #include "lpeg.h" #include "lyaml.h" @@ -73,6 +75,15 @@ void falco_engine::load_rules(const string &rules_content, bool verbose, bool al { m_rules = new falco_rules(m_inspector, this, m_ls); } + + // Note that falco_formats is added to both the lua state used + // by the falco engine as well as the separate lua state used + // by falco outputs. Within the engine, only + // formats.formatter is used, so we can unconditionally set + // json_output to false. + bool json_output = false; + falco_formats::init(m_inspector, m_ls, json_output); + m_rules->load_rules(rules_content, verbose, all_events, m_extra, m_replace_container_info); } diff --git a/userspace/falco/formats.cpp b/userspace/engine/formats.cpp similarity index 94% rename from userspace/falco/formats.cpp rename to userspace/engine/formats.cpp index d3ef1f00..625f9cbf 100644 --- a/userspace/falco/formats.cpp +++ b/userspace/engine/formats.cpp @@ -39,7 +39,7 @@ void falco_formats::init(sinsp* inspector, lua_State *ls, bool json_output) s_inspector = inspector; s_json_output = json_output; - luaL_openlib(ls, "falco", ll_falco, 0); + luaL_openlib(ls, "formats", ll_falco, 0); } int falco_formats::formatter(lua_State *ls) @@ -52,7 +52,7 @@ int falco_formats::formatter(lua_State *ls) } catch(sinsp_exception& e) { - throw falco_exception("Invalid output format '" + format + "'.\n"); + luaL_error(ls, "Invalid output format '%s': '%s'", format.c_str(), e.what()); } lua_pushlightuserdata(ls, formatter); @@ -64,14 +64,14 @@ int falco_formats::free_formatter(lua_State *ls) { if (!lua_islightuserdata(ls, -1)) { - throw falco_exception("Invalid argument passed to free_formatter"); + luaL_error(ls, "Invalid argument passed to free_formatter"); } sinsp_evt_formatter *formatter = (sinsp_evt_formatter *) lua_topointer(ls, 1); delete(formatter); - return 1; + return 0; } int falco_formats::format_event (lua_State *ls) diff --git a/userspace/falco/formats.h b/userspace/engine/formats.h similarity index 97% rename from userspace/falco/formats.h rename to userspace/engine/formats.h index 83a3609c..e5a2781a 100644 --- a/userspace/falco/formats.h +++ b/userspace/engine/formats.h @@ -43,7 +43,4 @@ class falco_formats static int format_event(lua_State *ls); static sinsp* s_inspector; - - private: - lua_State* m_ls; }; diff --git a/userspace/engine/lua/rule_loader.lua b/userspace/engine/lua/rule_loader.lua index fde97fa3..174bfd18 100644 --- a/userspace/engine/lua/rule_loader.lua +++ b/userspace/engine/lua/rule_loader.lua @@ -281,6 +281,12 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac v['output'] = v['output'].." "..extra end end + + -- Ensure that the output field is properly formatted by + -- creating a formatter from it. Any error will be thrown + -- up to the top level. + formatter = formats.formatter(v['output']) + formats.free_formatter(formatter) else error ("Unexpected type in load_rule: "..filter_ast.type) end diff --git a/userspace/falco/CMakeLists.txt b/userspace/falco/CMakeLists.txt index 9111bcfa..41988076 100644 --- a/userspace/falco/CMakeLists.txt +++ b/userspace/falco/CMakeLists.txt @@ -9,7 +9,7 @@ include_directories("${CURL_INCLUDE_DIR}") include_directories("${YAMLCPP_INCLUDE_DIR}") include_directories("${DRAIOS_DEPENDENCIES_DIR}/yaml-${DRAIOS_YAML_VERSION}/target/include") -add_executable(falco configuration.cpp formats.cpp logger.cpp falco_outputs.cpp falco.cpp) +add_executable(falco configuration.cpp logger.cpp falco_outputs.cpp falco.cpp) target_link_libraries(falco falco_engine sinsp) target_link_libraries(falco diff --git a/userspace/falco/falco_outputs.cpp b/userspace/falco/falco_outputs.cpp index 03f6b8a7..f77eb802 100644 --- a/userspace/falco/falco_outputs.cpp +++ b/userspace/falco/falco_outputs.cpp @@ -46,6 +46,9 @@ void falco_outputs::init(bool json_output) falco_common::init(m_lua_main_filename.c_str(), FALCO_SOURCE_LUA_DIR); + // Note that falco_formats is added to both the lua state used + // by the falco engine as well as the separate lua state used + // by falco outputs. falco_formats::init(m_inspector, m_ls, json_output); falco_logger::init(m_ls); diff --git a/userspace/falco/lua/output.lua b/userspace/falco/lua/output.lua index bd757916..e2ec0127 100644 --- a/userspace/falco/lua/output.lua +++ b/userspace/falco/lua/output.lua @@ -75,14 +75,14 @@ end function output_event(event, rule, priority, format) local level = level_of(priority) format = "*%evt.time: "..levels[level+1].." "..format - formatter = falco.formatter(format) - msg = falco.format_event(event, rule, levels[level+1], formatter) + formatter = formats.formatter(format) + msg = formats.format_event(event, rule, levels[level+1], formatter) for index,o in ipairs(outputs) do o.output(level, msg, o.config) end - falco.free_formatter(formatter) + formats.free_formatter(formatter) end function add_output(output_name, config)