From a34cbdcb749d697659acc20c303e3ac1339a796a Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Wed, 16 Oct 2024 13:02:22 +0200 Subject: [PATCH 1/3] new(userspace): added new `addOutput` json entry for plugin `get_field()` API. It suggests to Falco that some fields should be enforced to all compatible sources output. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/plugin.cpp | 12 ++++++ userspace/libsinsp/plugin.h | 10 +++++ userspace/libsinsp/test/plugins.ut.cpp | 41 ++++++++++++++++++- .../libsinsp/test/plugins/plugin_extract.cpp | 2 +- .../libsinsp/test/plugins/syscall_extract.cpp | 3 +- userspace/plugin/plugin_api.h | 8 ++-- 6 files changed, 70 insertions(+), 6 deletions(-) diff --git a/userspace/libsinsp/plugin.cpp b/userspace/libsinsp/plugin.cpp index 88a04479a2..bc8d8b7342 100644 --- a/userspace/libsinsp/plugin.cpp +++ b/userspace/libsinsp/plugin.cpp @@ -513,6 +513,18 @@ bool sinsp_plugin::resolve_dylib_symbols(std::string& errstr) { } } + const Json::Value& jvoutput = root[j].get("addOutput", Json::Value::null); + if(!jvoutput.isNull()) { + if(!jvoutput.isBool()) { + throw sinsp_exception(string("error in plugin ") + name() + ": field " + fname + + " addOutput property is not boolean "); + } + + if(jvoutput.asBool()) { + m_output_fields.emplace("%" + fname); + } + } + resolve_dylib_field_arg(root[j].get("arg", Json::Value::null), tf); const Json::Value& jvProperties = root[j].get("properties", Json::Value::null); diff --git a/userspace/libsinsp/plugin.h b/userspace/libsinsp/plugin.h index 155860e096..9c35dfaeee 100644 --- a/userspace/libsinsp/plugin.h +++ b/userspace/libsinsp/plugin.h @@ -116,6 +116,7 @@ class sinsp_plugin { m_scap_source_plugin(), m_fields_info(), m_fields(), + m_output_fields(), m_extract_event_sources(), m_extract_event_codes(), m_parse_event_sources(), @@ -173,6 +174,14 @@ class sinsp_plugin { std::vector list_open_params() const; /** Field Extraction **/ + inline const std::unordered_set& append_outputs_fields(std::string& source) const { + static std::unordered_set empty_set; + if(m_extract_event_sources.find(source) != m_extract_event_sources.end()) { + return m_output_fields; + } + return empty_set; + } + inline const std::unordered_set& extract_event_sources() const { return m_extract_event_sources; } @@ -236,6 +245,7 @@ class sinsp_plugin { /** Field Extraction **/ filter_check_info m_fields_info; std::vector m_fields; + std::unordered_set m_output_fields; std::unordered_set m_extract_event_sources; libsinsp::events::set m_extract_event_codes; diff --git a/userspace/libsinsp/test/plugins.ut.cpp b/userspace/libsinsp/test/plugins.ut.cpp index 78b48c34b8..799dc13c82 100644 --- a/userspace/libsinsp/test/plugins.ut.cpp +++ b/userspace/libsinsp/test/plugins.ut.cpp @@ -186,6 +186,23 @@ TEST_F(sinsp_with_test_input, plugin_syscall_extract) { ASSERT_FALSE(field_has_value(evt, "sample.evt_count", pl_flist)); ASSERT_EQ(get_field_as_string(evt, "sample.tick", pl_flist), "false"); + // Since `%sample.is_open` field was requested by the plugin as an addOutput field, + // its value should be present in the output. + std::string output_fmt; + bool first = true; + for(const auto& output_field : pl->append_outputs_fields(syscall_source_name)) { + if(!first) { + output_fmt += " "; + } else { + first = false; + } + output_fmt += output_field; + } + auto formatter = sinsp_evt_formatter(&m_inspector, output_fmt, pl_flist); + std::string output; + ASSERT_TRUE(formatter.tostring(evt, output)); + ASSERT_EQ(output, "1"); + // Check rhs filter checks support on plugins // Check on strings @@ -229,6 +246,9 @@ TEST_F(sinsp_with_test_input, plugin_syscall_extract) { ASSERT_FALSE(field_has_value(evt, "sample.evt_count", pl_flist)); ASSERT_EQ(get_field_as_string(evt, "sample.tick", pl_flist), "false"); + ASSERT_TRUE(formatter.tostring(evt, output)); + ASSERT_EQ(output, "0"); + // should extract NULL for ignored event codes // `PPME_SYSCALL_OPEN_BY_HANDLE_AT_X` is an ignored event, see plugin_get_extract_event_types evt = add_event_advance_ts(increasing_ts(), @@ -346,15 +366,34 @@ TEST_F(sinsp_with_test_input, plugin_custom_source) { // the GENERIC platform type does not fill in machine_info ASSERT_EQ(m_inspector.get_machine_info()->num_cpus, 0); + auto evt_source = src_pl->event_source(); auto evt = next_event(); ASSERT_NE(evt, nullptr); ASSERT_EQ(evt->get_type(), PPME_PLUGINEVENT_E); ASSERT_EQ(evt->get_source_idx(), 1); ASSERT_EQ(evt->get_tid(), (uint64_t)-1); - ASSERT_EQ(std::string(evt->get_source_name()), src_pl->event_source()); + ASSERT_EQ(std::string(evt->get_source_name()), evt_source); ASSERT_FALSE(field_has_value(evt, "fd.name", filterlist)); ASSERT_EQ(get_field_as_string(evt, "evt.pluginname", filterlist), src_pl->name()); ASSERT_EQ(get_field_as_string(evt, "sample.hello", filterlist), "hello world"); + + // Since `%sample.hello` field was requested by the plugin as an addOutput field, + // its value should be present in the output. + std::string output_fmt; + bool first = true; + for(const auto& output_field : ext_pl->append_outputs_fields(evt_source)) { + if(!first) { + output_fmt += " "; + } else { + first = false; + } + output_fmt += output_field; + } + auto formatter = sinsp_evt_formatter(&m_inspector, output_fmt, filterlist); + std::string output; + ASSERT_TRUE(formatter.tostring(evt, output)); + ASSERT_EQ(output, "hello world"); + ASSERT_EQ(next_event(), nullptr); // EOF is expected } diff --git a/userspace/libsinsp/test/plugins/plugin_extract.cpp b/userspace/libsinsp/test/plugins/plugin_extract.cpp index 911092e24c..08e36ccbdd 100644 --- a/userspace/libsinsp/test/plugins/plugin_extract.cpp +++ b/userspace/libsinsp/test/plugins/plugin_extract.cpp @@ -64,7 +64,7 @@ const char* plugin_get_contact() { const char* plugin_get_fields() { return "[" "{\"type\": \"string\", \"name\": \"sample.hello\", \"desc\": \"A constant hello world " - "string\"}" + "string\", \"addOutput\": true}" "]"; } diff --git a/userspace/libsinsp/test/plugins/syscall_extract.cpp b/userspace/libsinsp/test/plugins/syscall_extract.cpp index 03f57f8cda..6672d1a618 100644 --- a/userspace/libsinsp/test/plugins/syscall_extract.cpp +++ b/userspace/libsinsp/test/plugins/syscall_extract.cpp @@ -86,7 +86,8 @@ const char* plugin_get_fields() { { "type": "uint64", "name": "sample.is_open", - "desc": "Value is 1 if event is of open family" + "desc": "Value is 1 if event is of open family", + "addOutput": true }, { "type": "uint64", diff --git a/userspace/plugin/plugin_api.h b/userspace/plugin/plugin_api.h index 9ec187d47c..1152b4aab1 100644 --- a/userspace/plugin/plugin_api.h +++ b/userspace/plugin/plugin_api.h @@ -29,7 +29,7 @@ extern "C" { // // todo(jasondellaluce): when/if major changes to v4, check and solve all todos #define PLUGIN_API_VERSION_MAJOR 3 -#define PLUGIN_API_VERSION_MINOR 7 +#define PLUGIN_API_VERSION_MINOR 8 #define PLUGIN_API_VERSION_PATCH 0 // @@ -843,7 +843,7 @@ typedef struct { // "name": a string with a name for the field // "type": one of "string", "uint64", "bool", "reltime", "abstime", // "ipaddr", "ipnet" - // "isList: (optional) If present and set to true, notes + // "isList: (optional) if present and set to true, notes // that the field extracts a list of values. // "arg": (optional) if present, notes that the field can accept // an argument e.g. field[arg]. More precisely, the following @@ -860,9 +860,11 @@ typedef struct { // display the field instead of the name. Used in tools // like wireshark. // "desc": a string with a description of the field + // "addOutput": (optional) if true, suggest this field to be appended to the + // output string for compatible event sources. // Example return value: // [ - // {"type": "uint64", "name": "field1", "desc": "Describing field 1"}, + // {"type": "uint64", "name": "field1", "desc": "Describing field 1", "addOutput": true}, // {"type": "string", "name": "field2", "arg": {"isRequired": true, "isIndex": true}, // "desc": "Describing field 2"}, // ] From 0195b5490d91f7708f8a19c26f9a4f7825ed6fb9 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 21 Oct 2024 14:57:39 +0200 Subject: [PATCH 2/3] chore(userspace/libsinsp): add `EPF_FORMAT_SUGGESTED` filtercheck_field flag. Signed-off-by: Federico Di Pierro Co-authored-by: Jason Dellaluce --- userspace/libsinsp/filter_field.h | 6 ++++++ userspace/libsinsp/plugin.cpp | 4 +++- userspace/libsinsp/plugin.h | 15 +++++++++------ userspace/libsinsp/test/plugins.ut.cpp | 8 ++++---- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/userspace/libsinsp/filter_field.h b/userspace/libsinsp/filter_field.h index 95abb635d1..dcecb209c0 100644 --- a/userspace/libsinsp/filter_field.h +++ b/userspace/libsinsp/filter_field.h @@ -50,6 +50,7 @@ enum filtercheck_field_flags { 1 << 13, ///< data pointers extracted by this field may change across subsequent ///< extractions (even of other fields), which makes them unsafe to be used ///< with filter caching or field-to-field comparisons + EPF_FORMAT_SUGGESTED = 1 << 14, ///< this field is suggested to be used as output field }; /** @@ -105,6 +106,11 @@ struct filtercheck_field_info { // through a memory buffer copy (e.g. with a FTR_STORAGE transformer) // inline bool is_ptr_unstable() const { return m_flags & EPF_NO_PTR_STABILITY; } + + // + // Returns true if this field is a suggested as output + // + inline bool is_format_suggested() const { return m_flags & EPF_FORMAT_SUGGESTED; } }; /** diff --git a/userspace/libsinsp/plugin.cpp b/userspace/libsinsp/plugin.cpp index bc8d8b7342..8f76e6f389 100644 --- a/userspace/libsinsp/plugin.cpp +++ b/userspace/libsinsp/plugin.cpp @@ -521,7 +521,9 @@ bool sinsp_plugin::resolve_dylib_symbols(std::string& errstr) { } if(jvoutput.asBool()) { - m_output_fields.emplace("%" + fname); + tf.m_flags = (filtercheck_field_flags)((int)tf.m_flags | + (int)filtercheck_field_flags:: + EPF_FORMAT_SUGGESTED); } } diff --git a/userspace/libsinsp/plugin.h b/userspace/libsinsp/plugin.h index 9c35dfaeee..2fa239e59a 100644 --- a/userspace/libsinsp/plugin.h +++ b/userspace/libsinsp/plugin.h @@ -116,7 +116,6 @@ class sinsp_plugin { m_scap_source_plugin(), m_fields_info(), m_fields(), - m_output_fields(), m_extract_event_sources(), m_extract_event_codes(), m_parse_event_sources(), @@ -174,12 +173,17 @@ class sinsp_plugin { std::vector list_open_params() const; /** Field Extraction **/ - inline const std::unordered_set& append_outputs_fields(std::string& source) const { - static std::unordered_set empty_set; + inline std::unordered_set suggested_output_formats( + const std::string& source) const { + std::unordered_set output_fields; if(m_extract_event_sources.find(source) != m_extract_event_sources.end()) { - return m_output_fields; + for(const auto& field : m_fields) { + if(field.is_format_suggested()) { + output_fields.emplace("%" + field.m_name); + } + } } - return empty_set; + return output_fields; } inline const std::unordered_set& extract_event_sources() const { @@ -245,7 +249,6 @@ class sinsp_plugin { /** Field Extraction **/ filter_check_info m_fields_info; std::vector m_fields; - std::unordered_set m_output_fields; std::unordered_set m_extract_event_sources; libsinsp::events::set m_extract_event_codes; diff --git a/userspace/libsinsp/test/plugins.ut.cpp b/userspace/libsinsp/test/plugins.ut.cpp index 799dc13c82..30b9de7743 100644 --- a/userspace/libsinsp/test/plugins.ut.cpp +++ b/userspace/libsinsp/test/plugins.ut.cpp @@ -190,13 +190,13 @@ TEST_F(sinsp_with_test_input, plugin_syscall_extract) { // its value should be present in the output. std::string output_fmt; bool first = true; - for(const auto& output_field : pl->append_outputs_fields(syscall_source_name)) { + for(const auto& fmt : pl->suggested_output_formats(syscall_source_name)) { if(!first) { output_fmt += " "; } else { first = false; } - output_fmt += output_field; + output_fmt += fmt; } auto formatter = sinsp_evt_formatter(&m_inspector, output_fmt, pl_flist); std::string output; @@ -381,13 +381,13 @@ TEST_F(sinsp_with_test_input, plugin_custom_source) { // its value should be present in the output. std::string output_fmt; bool first = true; - for(const auto& output_field : ext_pl->append_outputs_fields(evt_source)) { + for(const auto& fmt : ext_pl->suggested_output_formats(evt_source)) { if(!first) { output_fmt += " "; } else { first = false; } - output_fmt += output_field; + output_fmt += fmt; } auto formatter = sinsp_evt_formatter(&m_inspector, output_fmt, filterlist); std::string output; From 09995e339c18e42f4824ee451fef17cf8f988948 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 21 Oct 2024 15:53:50 +0200 Subject: [PATCH 3/3] cleanup(userspace/libsinsp): drop plugin-specific `suggested_output_formats` API. Signed-off-by: Federico Di Pierro Co-authored-by: Jason Dellaluce --- userspace/libsinsp/plugin.h | 13 ------ userspace/libsinsp/test/plugins.ut.cpp | 61 +++++++++----------------- 2 files changed, 21 insertions(+), 53 deletions(-) diff --git a/userspace/libsinsp/plugin.h b/userspace/libsinsp/plugin.h index 2fa239e59a..155860e096 100644 --- a/userspace/libsinsp/plugin.h +++ b/userspace/libsinsp/plugin.h @@ -173,19 +173,6 @@ class sinsp_plugin { std::vector list_open_params() const; /** Field Extraction **/ - inline std::unordered_set suggested_output_formats( - const std::string& source) const { - std::unordered_set output_fields; - if(m_extract_event_sources.find(source) != m_extract_event_sources.end()) { - for(const auto& field : m_fields) { - if(field.is_format_suggested()) { - output_fields.emplace("%" + field.m_name); - } - } - } - return output_fields; - } - inline const std::unordered_set& extract_event_sources() const { return m_extract_event_sources; } diff --git a/userspace/libsinsp/test/plugins.ut.cpp b/userspace/libsinsp/test/plugins.ut.cpp index 30b9de7743..f75474f873 100644 --- a/userspace/libsinsp/test/plugins.ut.cpp +++ b/userspace/libsinsp/test/plugins.ut.cpp @@ -155,6 +155,16 @@ TEST_F(sinsp_with_test_input, plugin_syscall_extract) { // This plugin tells that it can receive `syscall` events add_plugin_filterchecks(&m_inspector, pl, sinsp_syscall_event_source_name, pl_flist); + // Since `%ample.is_open` field was requested by the plugin as an addOutput field, + // its filtercheck should have the EPF_FORMAT_SUGGESTED flag.. + std::vector fields; + pl_flist.get_all_fields(fields); + for(const auto& field : fields) { + if(field->m_name == "sample.is_open") { + ASSERT_TRUE(field->m_flags & EPF_FORMAT_SUGGESTED); + } + } + // Open the inspector in test mode add_default_init_thread(); open_inspector(); @@ -186,23 +196,6 @@ TEST_F(sinsp_with_test_input, plugin_syscall_extract) { ASSERT_FALSE(field_has_value(evt, "sample.evt_count", pl_flist)); ASSERT_EQ(get_field_as_string(evt, "sample.tick", pl_flist), "false"); - // Since `%sample.is_open` field was requested by the plugin as an addOutput field, - // its value should be present in the output. - std::string output_fmt; - bool first = true; - for(const auto& fmt : pl->suggested_output_formats(syscall_source_name)) { - if(!first) { - output_fmt += " "; - } else { - first = false; - } - output_fmt += fmt; - } - auto formatter = sinsp_evt_formatter(&m_inspector, output_fmt, pl_flist); - std::string output; - ASSERT_TRUE(formatter.tostring(evt, output)); - ASSERT_EQ(output, "1"); - // Check rhs filter checks support on plugins // Check on strings @@ -246,9 +239,6 @@ TEST_F(sinsp_with_test_input, plugin_syscall_extract) { ASSERT_FALSE(field_has_value(evt, "sample.evt_count", pl_flist)); ASSERT_EQ(get_field_as_string(evt, "sample.tick", pl_flist), "false"); - ASSERT_TRUE(formatter.tostring(evt, output)); - ASSERT_EQ(output, "0"); - // should extract NULL for ignored event codes // `PPME_SYSCALL_OPEN_BY_HANDLE_AT_X` is an ignored event, see plugin_get_extract_event_types evt = add_event_advance_ts(increasing_ts(), @@ -366,34 +356,25 @@ TEST_F(sinsp_with_test_input, plugin_custom_source) { // the GENERIC platform type does not fill in machine_info ASSERT_EQ(m_inspector.get_machine_info()->num_cpus, 0); - auto evt_source = src_pl->event_source(); + // Since `%sample.hello` field was requested by the plugin as an addOutput field, + // its value should be present in the output. + std::vector fields; + filterlist.get_all_fields(fields); + for(const auto& field : fields) { + if(field->m_name == "sample.hello") { + ASSERT_TRUE(field->m_flags & EPF_FORMAT_SUGGESTED); + } + } + auto evt = next_event(); ASSERT_NE(evt, nullptr); ASSERT_EQ(evt->get_type(), PPME_PLUGINEVENT_E); ASSERT_EQ(evt->get_source_idx(), 1); ASSERT_EQ(evt->get_tid(), (uint64_t)-1); - ASSERT_EQ(std::string(evt->get_source_name()), evt_source); + ASSERT_EQ(std::string(evt->get_source_name()), src_pl->event_source()); ASSERT_FALSE(field_has_value(evt, "fd.name", filterlist)); ASSERT_EQ(get_field_as_string(evt, "evt.pluginname", filterlist), src_pl->name()); ASSERT_EQ(get_field_as_string(evt, "sample.hello", filterlist), "hello world"); - - // Since `%sample.hello` field was requested by the plugin as an addOutput field, - // its value should be present in the output. - std::string output_fmt; - bool first = true; - for(const auto& fmt : ext_pl->suggested_output_formats(evt_source)) { - if(!first) { - output_fmt += " "; - } else { - first = false; - } - output_fmt += fmt; - } - auto formatter = sinsp_evt_formatter(&m_inspector, output_fmt, filterlist); - std::string output; - ASSERT_TRUE(formatter.tostring(evt, output)); - ASSERT_EQ(output, "hello world"); - ASSERT_EQ(next_event(), nullptr); // EOF is expected }