[WIP] cleanup(sinsp)!: enforce some runtime checks and remove an extra space from evt.args

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

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++)

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.

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());
 			}
 
@@ -1159,6 +1157,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;

userspace/libsinsp/test/filterchecks/evt.cpp

@@ -16,7 +16,7 @@ limitations under the License.
 
 */
 
-#include <test/helpers/threads_helpers.h>
+#include <test/sinsp_with_test_input.h>
 
 TEST_F(sinsp_with_test_input, EVT_FILTER_is_open_create)
 {
@@ -44,26 +44,91 @@ 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;
+
+	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");
+
+	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));
+
+	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);
+
+	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");
 }
 
-TEST_F(sinsp_with_test_input, EVT_FILTER_rawarg_str)
+TEST_F(sinsp_with_test_input, EVT_FILTER_check_evt_arg_uid)
 {
 	add_default_init_thread();
-
 	open_inspector();
 
-	std::string path = "/home/file.txt";
+	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");
 
-	// 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);
-}
+	// 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"), "<NA>");
+	ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), "<NA>");
+	ASSERT_EQ(get_field_as_string(evt, "evt.args"), "uid=5(<NA>)");
+
+	// 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"), "<NA>");
+	ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), "<NA>");
+	ASSERT_EQ(get_field_as_string(evt, "evt.args"), "uid=5(<NA>)");
+}

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 <gtest/gtest.h>
+#include <test/sinsp_with_test_input.h>
+#include <test/test_utils.h>
+
+// 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<uint8_t> 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, "");
+}

userspace/libsinsp/test/test_utils.cpp

@@ -20,24 +20,14 @@ limitations under the License.
 
 #include <cstring>
 
-#if defined(__linux__)
-#include <linux/un.h>
-#else
-#if !defined(_WIN32)
-#include <sys/un.h>
-# endif //_WIN32
-#ifndef UNIX_PATH_MAX
-#define UNIX_PATH_MAX 108
-#endif
-#endif
-
 #if !defined(_WIN32)
 #include <arpa/inet.h>
 #endif //_WIN32
 #include <stdint.h>
 
 #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<std::string> list)
@@ -262,6 +261,31 @@ std::vector<uint8_t> pack_socktuple(sockaddr *src, sockaddr *dest)
 
 	return res;
 }
+
+std::vector<uint8_t> pack_unix_socktuple(uint64_t scr_pointer, uint64_t dst_pointer, std::string unix_path)
+{
+	std::vector<uint8_t> 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