From dd65717d3cba1b1b442eedfef9fcedf902989e37 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Sun, 17 Dec 2023 16:48:42 +0100 Subject: [PATCH 1/5] style(modern_bpf): reword a comment Signed-off-by: Andrea Terzolo --- driver/modern_bpf/helpers/store/auxmap_store_params.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/driver/modern_bpf/helpers/store/auxmap_store_params.h b/driver/modern_bpf/helpers/store/auxmap_store_params.h index 5f35c221dc..885de4c3eb 100644 --- a/driver/modern_bpf/helpers/store/auxmap_store_params.h +++ b/driver/modern_bpf/helpers/store/auxmap_store_params.h @@ -683,11 +683,18 @@ static __always_inline void auxmap__store_socktuple_param(struct auxiliary_map * } unsigned long start_reading_point; - /* We have to skip the two bytes of socket family. */ char first_path_byte = *(char *)path; if(first_path_byte == '\0') { - /* This is an abstract socket address, we need to skip the initial `\0`. */ + /* Please note exceptions in the `sun_path`: + * Taken from: https://man7.org/linux/man-pages/man7/unix.7.html + * + * An `abstract socket address` is distinguished (from a + * pathname socket) by the fact that sun_path[0] is a null byte + * ('\0'). + * + * So in this case, we need to skip the initial `\0`. + */ start_reading_point = (unsigned long)path + 1; } else From 2af68feaef31b8edf8741f6bc62d4c6c69561999 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Wed, 20 Dec 2023 16:12:51 +0100 Subject: [PATCH 2/5] new(tests): add a test for unix sockets Signed-off-by: Andrea Terzolo --- .../libsinsp/test/parsers/parse_connect.cpp | 93 +++++++++++++++++++ userspace/libsinsp/test/test_utils.cpp | 46 ++++++--- userspace/libsinsp/test/test_utils.h | 13 +++ 3 files changed, 141 insertions(+), 11 deletions(-) create mode 100644 userspace/libsinsp/test/parsers/parse_connect.cpp diff --git a/userspace/libsinsp/test/parsers/parse_connect.cpp b/userspace/libsinsp/test/parsers/parse_connect.cpp new file mode 100644 index 0000000000..c3302825b3 --- /dev/null +++ b/userspace/libsinsp/test/parsers/parse_connect.cpp @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: Apache-2.0 +/* +Copyright (C) 2023 The Falco Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +*/ + +#include +#include +#include + +// Note: +// 1. We don't save the type of the unix socket: datagram or stream +// 2. Do we want to keep the tuple in this way `9c758d0f->9c758d0a /tmp/stream.sock`? +TEST_F(sinsp_with_test_input, CONNECT_parse_unix_socket) +{ + add_default_init_thread(); + open_inspector(); + + int64_t return_value = 0; + int64_t client_fd = 9; + + // We need the enter event because we store it and we use it in the exit one. + // We only store it, we don't create a fdinfo, if the enter event is missing + // we don't parse the exit one. + auto evt = add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SOCKET_SOCKET_E, 3, (uint32_t)PPM_AF_UNIX, + (uint32_t)SOCK_STREAM, (uint32_t)0); + auto fdinfo = evt->get_fd_info(); + ASSERT_FALSE(fdinfo); + + evt = add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SOCKET_SOCKET_X, 1, client_fd); + + /* FDINFO associated with the event */ + fdinfo = evt->get_fd_info(); + ASSERT_TRUE(fdinfo); + ASSERT_TRUE(fdinfo->is_unix_socket()); + // todo! do we want this? In the end a unix socket could be of type datagram or stream + ASSERT_EQ(fdinfo->get_l4proto(), scap_l4_proto::SCAP_L4_NA); + ASSERT_TRUE(fdinfo->is_role_none()); + ASSERT_FALSE(fdinfo->is_socket_connected()); + // The socket syscall doesn't populate the name of the socket + ASSERT_EQ(fdinfo->m_name, ""); + + /* FDINFO associated with the thread */ + auto init_tinfo = m_inspector.get_thread_ref(INIT_TID, false).get(); + ASSERT_TRUE(init_tinfo); + fdinfo = init_tinfo->get_fd(client_fd); + ASSERT_TRUE(fdinfo); + ASSERT_TRUE(fdinfo->is_unix_socket()); + ASSERT_EQ(fdinfo->get_l4proto(), scap_l4_proto::SCAP_L4_NA); + ASSERT_TRUE(fdinfo->is_role_none()); + ASSERT_FALSE(fdinfo->is_socket_connected()); + ASSERT_EQ(fdinfo->m_name, ""); + + // We don't need the enter event! + std::vector socktuple = test_utils::pack_unix_socktuple(0x9c758d0f, 0x9c758d0a, "/tmp/stream.sock"); + evt = add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SOCKET_CONNECT_X, 3, return_value, + scap_const_sized_buffer{socktuple.data(), socktuple.size()}, client_fd); + + /* FDINFO associated with the event */ + fdinfo = evt->get_fd_info(); + ASSERT_TRUE(fdinfo); + ASSERT_TRUE(fdinfo->is_unix_socket()); + ASSERT_EQ(fdinfo->get_l4proto(), scap_l4_proto::SCAP_L4_NA); + ASSERT_TRUE(fdinfo->is_role_client()); + ASSERT_TRUE(fdinfo->is_socket_connected()); + // Note: `9c758d0f` is the kernel pointer to the socket that performs the connection. + // `9c758d0a` is the kernel pointer to the socket that receives the connection. + ASSERT_EQ(fdinfo->m_name, "9c758d0f->9c758d0a /tmp/stream.sock"); + // we don't have code to populate this `m_name_raw` for sockets. + ASSERT_EQ(fdinfo->m_name_raw, ""); + + /* FDINFO associated with the thread */ + fdinfo = init_tinfo->get_fd(client_fd); + ASSERT_TRUE(fdinfo); + ASSERT_TRUE(fdinfo->is_unix_socket()); + ASSERT_EQ(fdinfo->get_l4proto(), scap_l4_proto::SCAP_L4_NA); + ASSERT_TRUE(fdinfo->is_role_client()); + ASSERT_TRUE(fdinfo->is_socket_connected()); + ASSERT_EQ(fdinfo->m_name, "9c758d0f->9c758d0a /tmp/stream.sock"); + ASSERT_EQ(fdinfo->m_name_raw, ""); +} diff --git a/userspace/libsinsp/test/test_utils.cpp b/userspace/libsinsp/test/test_utils.cpp index e401727b00..61daf73a35 100644 --- a/userspace/libsinsp/test/test_utils.cpp +++ b/userspace/libsinsp/test/test_utils.cpp @@ -20,17 +20,6 @@ limitations under the License. #include -#if defined(__linux__) -#include -#else -#if !defined(_WIN32) -#include -# endif //_WIN32 -#ifndef UNIX_PATH_MAX -#define UNIX_PATH_MAX 108 -#endif -#endif - #if !defined(_WIN32) #include #endif //_WIN32 @@ -38,6 +27,7 @@ limitations under the License. #include "ppm_events_public.h" #include "userspace_flag_helpers.h" +#include "strl.h" namespace test_utils { @@ -61,6 +51,15 @@ struct sockaddr_in6 fill_sockaddr_in6(int32_t ipv6_port, const char* ipv6_string inet_pton(AF_INET6, ipv6_string, &(sockaddr.sin6_addr)); return sockaddr; } + +struct sockaddr_un fill_sockaddr_un(const char* unix_path) +{ + struct sockaddr_un sockaddr; + memset(&sockaddr, 0, sizeof(sockaddr)); + sockaddr.sun_family = AF_UNIX; + strlcpy(sockaddr.sun_path, unix_path, UNIX_PATH_MAX); + return sockaddr; +} #endif //_WIN32 std::string to_null_delimited(const std::vector list) @@ -262,6 +261,31 @@ std::vector pack_socktuple(sockaddr *src, sockaddr *dest) return res; } + +std::vector pack_unix_socktuple(uint64_t scr_pointer, uint64_t dst_pointer, std::string unix_path) +{ + std::vector res; + + // Assert family. + res.push_back(PPM_AF_UNIX); + + // Scr pointer + for (size_t i = 0; i < sizeof(scr_pointer); ++i) + { + res.push_back(scr_pointer & 0xFF); + scr_pointer >>= 8; + } + + // Dest pointer + for (size_t i = 0; i < sizeof(dst_pointer); ++i) + { + res.push_back(dst_pointer & 0xFF); + dst_pointer >>= 8; + } + + res.insert(res.end(), unix_path.begin(), unix_path.end()); + return res; +} #endif //_WIN32 __EMSCRIPTEN__ } // namespace test_utils diff --git a/userspace/libsinsp/test/test_utils.h b/userspace/libsinsp/test/test_utils.h index 10d8ea81e8..2e2fb1c8b4 100644 --- a/userspace/libsinsp/test/test_utils.h +++ b/userspace/libsinsp/test/test_utils.h @@ -27,6 +27,17 @@ limitations under the License. #endif //_WIN32 #include +#if defined(__linux__) +#include +#else +#if !defined(_WIN32) +#include +# endif //_WIN32 +#ifndef UNIX_PATH_MAX +#define UNIX_PATH_MAX 108 +#endif +#endif + #define ASSERT_NAMES_EQ(a, b) \ { \ auto a1 = a; \ @@ -92,8 +103,10 @@ std::set unordered_set_to_ordered(std::unordered_set unordered_set); #if !defined(_WIN32) struct sockaddr_in fill_sockaddr_in(int32_t ipv4_port, const char* ipv4_string); struct sockaddr_in6 fill_sockaddr_in6(int32_t ipv6_port, const char* ipv6_string); +struct sockaddr_un fill_sockaddr_un(const char* unix_path); std::vector pack_sockaddr(sockaddr *sa); std::vector pack_socktuple(sockaddr *src, sockaddr *dest); +std::vector pack_unix_socktuple(uint64_t scr_pointer, uint64_t dst_pointer, std::string unix_path); #endif //_WIN32 void print_bytes(uint8_t *buf, size_t size); From dc57b7e6e9898c4884f9a83dd936acfadff5ab19 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Wed, 20 Dec 2023 16:13:37 +0100 Subject: [PATCH 3/5] fix(sinsp): remove an extra " " Signed-off-by: Andrea Terzolo --- userspace/libsinsp/sinsp_filtercheck_event.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/userspace/libsinsp/sinsp_filtercheck_event.cpp b/userspace/libsinsp/sinsp_filtercheck_event.cpp index c8e787f1d5..9c878cdad6 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_event.cpp @@ -1159,6 +1159,10 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo } } + if(!m_strstorage.empty()) + { + m_strstorage.pop_back(); + } RETURN_EXTRACT_STRING(m_strstorage); } break; From ea5cb972b3651f3a6ec00538ea76f4336a53fb4f Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Wed, 20 Dec 2023 16:14:07 +0100 Subject: [PATCH 4/5] update(sinsp)!: add some validation checks on `evt.` filters Signed-off-by: Andrea Terzolo --- userspace/libsinsp/event.cpp | 21 +++++++ userspace/libsinsp/event.h | 18 ++++++ .../libsinsp/sinsp_filtercheck_event.cpp | 8 +-- userspace/libsinsp/test/filterchecks/evt.cpp | 61 ++++++++++++++----- 4 files changed, 87 insertions(+), 21 deletions(-) diff --git a/userspace/libsinsp/event.cpp b/userspace/libsinsp/event.cpp index 74a31b5573..dee2996209 100644 --- a/userspace/libsinsp/event.cpp +++ b/userspace/libsinsp/event.cpp @@ -1827,6 +1827,27 @@ const char* sinsp_evt::get_param_as_str(uint32_t id, OUT const char** resolved_s return &m_paramstr_storage[0]; } +void sinsp_evt::check_param_name_exists(const std::string &name) +{ + for(uint32_t i = 0; i < get_num_params(); i++) + { + if(name.compare(get_param_name(i)) == 0) + { + return; + } + } + + throw sinsp_exception("Event '" + std::string(this->get_name()) + "' has no parameters called '" + name +"'. Double check the documentation."); +} + +void sinsp_evt::check_param_id_exists(int32_t id) +{ + if(id >= (int32_t)this->get_num_params()) + { + throw sinsp_exception("Event '" + std::string(this->get_name()) + "' has '" + std::to_string(this->get_num_params()) +"' parameters, so '" + std::to_string(id) +"' cannot be used as an index. Only indexes '< " + std::to_string(this->get_num_params())+ "' are allowed."); + } +} + std::string sinsp_evt::get_param_value_str(const std::string &name, bool resolved) { for(uint32_t i = 0; i < get_num_params(); i++) diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h index e759ac4a94..55e8a2fded 100644 --- a/userspace/libsinsp/event.h +++ b/userspace/libsinsp/event.h @@ -439,6 +439,24 @@ class SINSP_PUBLIC sinsp_evt : public gen_event */ std::string get_param_value_str(const std::string& name, bool resolved = true); + /*! + \brief Check if a parameter is exposed by an event. + Throw an exception in case the parameter doesn't exist. + todo! This check should be done when loading rules not at runtime. + + \param name The parameter name. + */ + void check_param_name_exists(const std::string &name); + + /*! + \brief Check if a parameter id is exposed by an event. + Throw an exception in case the parameter doesn't exist. + todo! This check should be done when loading rules not at runtime. + + \param id The parameter id. + */ + void check_param_id_exists(int32_t id); + /*! \brief Return the event's category, based on the event type and the FD on which the event operates. diff --git a/userspace/libsinsp/sinsp_filtercheck_event.cpp b/userspace/libsinsp/sinsp_filtercheck_event.cpp index 9c878cdad6..4981d48e47 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_event.cpp @@ -1076,6 +1076,7 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo case TYPE_CPU: RETURN_EXTRACT_VAR(evt->m_cpuid); case TYPE_ARGRAW: + evt->check_param_name_exists(m_argname); return extract_argraw(evt, len, m_arginfo->name); break; case TYPE_ARGSTR: @@ -1087,15 +1088,12 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo if(m_argid != -1) { - if(m_argid >= (int32_t)evt->get_num_params()) - { - return NULL; - } - + evt->check_param_id_exists(m_argid); argstr = evt->get_param_as_str(m_argid, &resolved_argstr, m_inspector->get_buffer_format()); } else { + evt->check_param_name_exists(m_argname); argstr = evt->get_param_value_str(m_argname.c_str(), &resolved_argstr, m_inspector->get_buffer_format()); } diff --git a/userspace/libsinsp/test/filterchecks/evt.cpp b/userspace/libsinsp/test/filterchecks/evt.cpp index 1fb3f13026..f4392b5645 100644 --- a/userspace/libsinsp/test/filterchecks/evt.cpp +++ b/userspace/libsinsp/test/filterchecks/evt.cpp @@ -16,7 +16,7 @@ limitations under the License. */ -#include +#include TEST_F(sinsp_with_test_input, EVT_FILTER_is_open_create) { @@ -44,26 +44,55 @@ TEST_F(sinsp_with_test_input, EVT_FILTER_is_open_create) ASSERT_EQ(evt->m_fdinfo->m_openflags, PPM_O_RDWR | PPM_O_CREAT | PPM_O_F_CREATED); } -TEST_F(sinsp_with_test_input, EVT_FILTER_rawarg_int) +// Check all filterchecks `evt.arg*` +TEST_F(sinsp_with_test_input, EVT_FILTER_check_evt_arg) { add_default_init_thread(); - open_inspector(); - sinsp_evt* evt = add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_SETUID_E, 1, (uint32_t)1000); - ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.uid"), "1000"); -} + std::string target = "sym"; + std::string linkpath = "/new/sym"; + int64_t err = 3; -TEST_F(sinsp_with_test_input, EVT_FILTER_rawarg_str) -{ - add_default_init_thread(); + auto evt = add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SYSCALL_SYMLINK_X, 3, err, target.c_str(), + linkpath.c_str()); + ASSERT_EQ(get_field_as_string(evt, "evt.type"), "symlink"); - open_inspector(); + ASSERT_EQ(get_field_as_string(evt, "evt.arg.res"), std::to_string(err)); + ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), std::to_string(err)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.res"), std::to_string(err)); - std::string path = "/home/file.txt"; + ASSERT_EQ(get_field_as_string(evt, "evt.arg.target"), target); + ASSERT_EQ(get_field_as_string(evt, "evt.arg[1]"), target); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.target"), target); - // In the enter event we don't send the `PPM_O_F_CREATED` - sinsp_evt* evt = add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_OPEN_E, 3, path.c_str(), - (uint32_t)0, (uint32_t)0); - ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.name"), path); -} \ No newline at end of file + ASSERT_EQ(get_field_as_string(evt, "evt.arg.linkpath"), linkpath); + ASSERT_EQ(get_field_as_string(evt, "evt.arg[2]"), linkpath); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.linkpath"), linkpath); + + // These fields should always have an argument + ASSERT_THROW(field_exists(evt, "evt.arg"), sinsp_exception); + ASSERT_THROW(field_exists(evt, "evt.rawarg"), sinsp_exception); + + // If the field is not contained in the event table we throw an exception + ASSERT_THROW(field_exists(evt, "evt.arg.not_exists"), sinsp_exception); + ASSERT_THROW(field_exists(evt, "evt.rawarg.not_exists"), sinsp_exception); + + // The validation is not during the argument extraction because we cannot access the event + // So here we return true. + ASSERT_TRUE(field_exists(evt, "evt.arg[126]")); + // Here we try to extract the field so we return an exception + ASSERT_THROW(field_has_value(evt, "evt.arg[126]"), sinsp_exception); + + // If the field is contained in the thread table but is not associated with this event we throw an + // exception during the extraction of the field, but we don't fail during the extraction of the argument. + // `.newpath` is not associated with `PPME_SYSCALL_SYMLINK_X` event. + ASSERT_TRUE(field_exists(evt, "evt.arg.newpath")); + ASSERT_THROW(field_has_value(evt, "evt.arg.newpath"), sinsp_exception); + + ASSERT_TRUE(field_exists(evt, "evt.rawarg.newpath")); + ASSERT_THROW(field_has_value(evt, "evt.rawarg.newpath"), sinsp_exception); + + // All the args of an event + ASSERT_EQ(get_field_as_string(evt, "evt.args"), "res=3 target=sym linkpath=/new/sym"); +} From ea29c4794515d03a51b96296c1c0917862c2dccf Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Thu, 21 Dec 2023 14:52:38 +0100 Subject: [PATCH 5/5] update(tests): rework a test on user id Signed-off-by: Andrea Terzolo --- userspace/libsinsp/test/filterchecks/evt.cpp | 36 ++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/userspace/libsinsp/test/filterchecks/evt.cpp b/userspace/libsinsp/test/filterchecks/evt.cpp index f4392b5645..8a8832de1f 100644 --- a/userspace/libsinsp/test/filterchecks/evt.cpp +++ b/userspace/libsinsp/test/filterchecks/evt.cpp @@ -96,3 +96,39 @@ TEST_F(sinsp_with_test_input, EVT_FILTER_check_evt_arg) // All the args of an event ASSERT_EQ(get_field_as_string(evt, "evt.args"), "res=3 target=sym linkpath=/new/sym"); } + +TEST_F(sinsp_with_test_input, EVT_FILTER_check_evt_arg_uid) +{ + add_default_init_thread(); + open_inspector(); + + uint32_t user_id = 5; + std::string container_id = ""; + auto evt = add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SYSCALL_SETUID_E, 1, user_id); + ASSERT_EQ(get_field_as_string(evt, "evt.type"), "setuid"); + + // The rawarg provides the field directly from the table. + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.uid"), std::to_string(user_id)); + + // The `evt.arg.uid` tries to find a user in the user table, in this + // case the user table is empty. + ASSERT_EQ(get_field_as_string(evt, "evt.arg.uid"), ""); + ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), ""); + ASSERT_EQ(get_field_as_string(evt, "evt.args"), "uid=5()"); + + // we are adding a user on the host so the `pid` parameter is not considered + ASSERT_TRUE(m_inspector.m_usergroup_manager.add_user(container_id, 0, user_id, 6, "test", "/test", "/bin/test")); + + // Now we should have the necessary info + ASSERT_EQ(get_field_as_string(evt, "evt.arg.uid"), "test"); + ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), "test"); + ASSERT_EQ(get_field_as_string(evt, "evt.args"), "uid=5(test)"); + + // We remove the user, and the fields should be empty again + m_inspector.m_usergroup_manager.rm_user(container_id, user_id); + ASSERT_FALSE(m_inspector.m_usergroup_manager.get_user(container_id, user_id)); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg.uid"), ""); + ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), ""); + ASSERT_EQ(get_field_as_string(evt, "evt.args"), "uid=5()"); +}