From 900ec2a4afd167b95c6e84aba9ef078e99a1539d Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 2 Dec 2024 10:21:18 +0100 Subject: [PATCH 1/4] fix(userspace/libsinsp): fixed uid 0 and gid 0 default values. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/threadinfo.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index 0b023da43b..b8d4fa80f7 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -535,8 +535,14 @@ void sinsp_threadinfo::set_user(uint32_t uid) { scap_userinfo* user = m_inspector->m_usergroup_manager.get_user(m_container_id, uid); if(!user) { auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin(); + std::string name, home; + // For uid 0 force set root related infos + if(uid == 0) { + name = "root"; + home = "/root"; + } m_inspector->m_usergroup_manager - .add_user(m_container_id, m_pid, uid, m_gid, {}, {}, {}, notify); + .add_user(m_container_id, m_pid, uid, m_gid, name, home, {}, notify); } } @@ -545,7 +551,11 @@ void sinsp_threadinfo::set_group(uint32_t gid) { scap_groupinfo* group = m_inspector->m_usergroup_manager.get_group(m_container_id, gid); if(!group) { auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin(); - m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, {}, notify); + std::string name; + if(gid == 0) { + name = "root"; + } + m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, name, notify); } } From d178abd345ba8fc8b91d4c771918c135547abb7a Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 2 Dec 2024 10:44:26 +0100 Subject: [PATCH 2/4] chore(userspace/libsinsp): added filtercheck user tests. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/test/filterchecks/user.cpp | 102 ++++++++++++++++++ userspace/libsinsp/threadinfo.cpp | 1 + 2 files changed, 103 insertions(+) create mode 100644 userspace/libsinsp/test/filterchecks/user.cpp diff --git a/userspace/libsinsp/test/filterchecks/user.cpp b/userspace/libsinsp/test/filterchecks/user.cpp new file mode 100644 index 0000000000..251742ab4c --- /dev/null +++ b/userspace/libsinsp/test/filterchecks/user.cpp @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 +/* +Copyright (C) 2024 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 + +TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry) { + add_default_init_thread(); + + auto& thread = m_threads.front(); + thread.uid = 1000; + thread.gid = 1000; + thread.loginuid = 0; + + open_inspector(); + + m_inspector.m_usergroup_manager.add_user("", INIT_TID, 1000, 1000, "foo", "/bar", "/bin/bash"); + + std::string path = "/home/file.txt"; + int64_t fd = 3; + + auto evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_OPEN_E, + fd, + path.c_str(), + (uint32_t)PPM_O_RDWR | PPM_O_CREAT, + (uint32_t)0); + ASSERT_EQ(get_field_as_string(evt, "user.uid"), "1000"); + ASSERT_EQ(get_field_as_string(evt, "user.loginuid"), "0"); + ASSERT_EQ(get_field_as_string(evt, "user.name"), "foo"); + ASSERT_EQ(get_field_as_string(evt, "user.homedir"), "/bar"); + ASSERT_EQ(get_field_as_string(evt, "user.shell"), "/bin/bash"); + // Loginname default at root for 0 uid without an user entry in user group manager. + ASSERT_EQ(get_field_as_string(evt, "user.loginname"), "root"); +} + +TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_unexistent_user_entry) { + add_default_init_thread(); + + open_inspector(); + + // The entry gets created when the inspector is opened and its threadtable created. + // Since default thread uid is 0, the entry is created with "root" name and "/root" homedir. + ASSERT_NE(m_inspector.m_usergroup_manager.get_user("", 0), nullptr); + + std::string path = "/home/file.txt"; + int64_t fd = 3; + + auto evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_OPEN_E, + fd, + path.c_str(), + (uint32_t)PPM_O_RDWR | PPM_O_CREAT, + (uint32_t)0); + + // For non-existent entries whose uid is 0, "root" and "/root" + // are automatically filled by threadinfo::get_user() method. + ASSERT_EQ(get_field_as_string(evt, "user.uid"), "0"); + ASSERT_EQ(get_field_as_string(evt, "user.name"), "root"); + ASSERT_EQ(get_field_as_string(evt, "user.homedir"), "/root"); +} + +TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry_without_metadata) { + add_default_init_thread(); + + open_inspector(); + + // Creating the entry in the user group manager will override + // the one created by the inspector threadtable initial load. + // Since we set empty metadata, we don't expect any metadata in the output fields. + m_inspector.m_usergroup_manager.add_user("", INIT_TID, 0, 0, "", "", ""); + + std::string path = "/home/file.txt"; + int64_t fd = 3; + + auto evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_OPEN_E, + fd, + path.c_str(), + (uint32_t)PPM_O_RDWR | PPM_O_CREAT, + (uint32_t)0); + ASSERT_EQ(get_field_as_string(evt, "user.uid"), "0"); + ASSERT_EQ(get_field_as_string(evt, "user.name"), ""); + ASSERT_EQ(get_field_as_string(evt, "user.homedir"), ""); +} diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index b8d4fa80f7..9deffaad79 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -551,6 +551,7 @@ void sinsp_threadinfo::set_group(uint32_t gid) { scap_groupinfo* group = m_inspector->m_usergroup_manager.get_group(m_container_id, gid); if(!group) { auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin(); + // For uid 0 force set root related infos std::string name; if(gid == 0) { name = "root"; From 3cee60f33f9f8107c0a61a66cd3dd6b115906824 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 2 Dec 2024 11:37:39 +0100 Subject: [PATCH 3/4] cleanup(userspace/libsinsp): clean up unused boolean flag in user group manager. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/sinsp.cpp | 3 +-- userspace/libsinsp/sinsp.h | 13 +------------ userspace/libsinsp/user.cpp | 1 - userspace/libsinsp/user.h | 2 -- 4 files changed, 2 insertions(+), 17 deletions(-) diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index 4d9d9bf4c7..09aa59e8ab 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -285,9 +285,8 @@ void sinsp::init() { m_inited = true; } -void sinsp::set_import_users(bool import_users, bool user_details) { +void sinsp::set_import_users(bool import_users) { m_usergroup_manager.m_import_users = import_users; - m_usergroup_manager.m_user_details_enabled = user_details; } /*=============================== OPEN METHODS ===============================*/ diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index 28c16f40b1..bb300c34ac 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -271,18 +271,12 @@ class SINSP_PUBLIC sinsp : public capture_stats_source { creating them can increase the startup time. Moreover, they contain information that could be privacy sensitive. - \param user_details if set to false, no extended user information will be - stored in sinsp_threadinfo, only user id/group id will be available. By - default thread information is enriched with the full set of user - information, i.e. name, homedir, shell, group name. The parameter - controls this behavior, an can be used to reduce memory footprint. - \note default behavior is import_users=true, user_details=true @throws a sinsp_exception containing the error string is thrown in case of failure. */ - void set_import_users(bool import_users, bool user_details = true); + void set_import_users(bool import_users); /*! \brief temporarily pauses event capture. @@ -738,11 +732,6 @@ class SINSP_PUBLIC sinsp : public capture_stats_source { */ inline bool is_debug_enabled() const { return m_isdebug_enabled; } - /*! - \brief Returns true if extended user information is collected. - */ - inline bool is_user_details_enabled() { return m_usergroup_manager.m_user_details_enabled; } - /*! \brief Set a flag indicating if the command line requested to show container information. diff --git a/userspace/libsinsp/user.cpp b/userspace/libsinsp/user.cpp index 6ae8af3794..69c7b5daa1 100644 --- a/userspace/libsinsp/user.cpp +++ b/userspace/libsinsp/user.cpp @@ -109,7 +109,6 @@ using namespace std; // clang-format off sinsp_usergroup_manager::sinsp_usergroup_manager(sinsp* inspector) : m_import_users(true) - , m_user_details_enabled(true) , m_last_flush_time_ns(0) , m_inspector(inspector) , m_host_root(m_inspector->get_host_root()) diff --git a/userspace/libsinsp/user.h b/userspace/libsinsp/user.h index 12c5436596..4151c74858 100644 --- a/userspace/libsinsp/user.h +++ b/userspace/libsinsp/user.h @@ -152,8 +152,6 @@ class sinsp_usergroup_manager { // bool m_import_users; - bool m_user_details_enabled; - private: scap_userinfo *add_host_user(uint32_t uid, uint32_t gid, From 451a3d9252c28ea3da382cd26485ce3229a8d051 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 2 Dec 2024 14:25:29 +0100 Subject: [PATCH 4/4] fix(userspace/libsinsp): get_user() and get_loginuser() need different static pointer. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/test/filterchecks/user.cpp | 57 ++++++++++++++++++- userspace/libsinsp/threadinfo.cpp | 44 +++++++------- userspace/libsinsp/threadinfo.h | 2 - 3 files changed, 79 insertions(+), 24 deletions(-) diff --git a/userspace/libsinsp/test/filterchecks/user.cpp b/userspace/libsinsp/test/filterchecks/user.cpp index 251742ab4c..b1425b855c 100644 --- a/userspace/libsinsp/test/filterchecks/user.cpp +++ b/userspace/libsinsp/test/filterchecks/user.cpp @@ -28,7 +28,7 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry) { open_inspector(); - m_inspector.m_usergroup_manager.add_user("", INIT_TID, 1000, 1000, "foo", "/bar", "/bin/bash"); + m_inspector.m_usergroup_manager.add_user("", INIT_TID, 1000, 1000, "foo", "/foo", "/bin/bash"); std::string path = "/home/file.txt"; int64_t fd = 3; @@ -43,13 +43,33 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry) { ASSERT_EQ(get_field_as_string(evt, "user.uid"), "1000"); ASSERT_EQ(get_field_as_string(evt, "user.loginuid"), "0"); ASSERT_EQ(get_field_as_string(evt, "user.name"), "foo"); + ASSERT_EQ(get_field_as_string(evt, "user.homedir"), "/foo"); + ASSERT_EQ(get_field_as_string(evt, "user.shell"), "/bin/bash"); + // Loginname default at root for 0 uid without an user entry in user group manager. + ASSERT_EQ(get_field_as_string(evt, "user.loginname"), "root"); + + // Now remove the user + m_inspector.m_usergroup_manager.rm_user("", 1000); + ASSERT_EQ(get_field_as_string(evt, "user.uid"), "1000"); + ASSERT_EQ(get_field_as_string(evt, "user.loginuid"), "0"); + ASSERT_EQ(get_field_as_string(evt, "user.name"), ""); + ASSERT_EQ(get_field_as_string(evt, "user.homedir"), ""); + ASSERT_EQ(get_field_as_string(evt, "user.shell"), ""); + // Loginname default at root for 0 uid without an user entry in user group manager. + ASSERT_EQ(get_field_as_string(evt, "user.loginname"), "root"); + + // Add back a new user + m_inspector.m_usergroup_manager.add_user("", INIT_TID, 1000, 1000, "bar", "/bar", "/bin/bash"); + ASSERT_EQ(get_field_as_string(evt, "user.uid"), "1000"); + ASSERT_EQ(get_field_as_string(evt, "user.loginuid"), "0"); + ASSERT_EQ(get_field_as_string(evt, "user.name"), "bar"); ASSERT_EQ(get_field_as_string(evt, "user.homedir"), "/bar"); ASSERT_EQ(get_field_as_string(evt, "user.shell"), "/bin/bash"); // Loginname default at root for 0 uid without an user entry in user group manager. ASSERT_EQ(get_field_as_string(evt, "user.loginname"), "root"); } -TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_unexistent_user_entry) { +TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_default_user_entry) { add_default_init_thread(); open_inspector(); @@ -58,6 +78,10 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_unexistent_user_entry) { // Since default thread uid is 0, the entry is created with "root" name and "/root" homedir. ASSERT_NE(m_inspector.m_usergroup_manager.get_user("", 0), nullptr); + // remove the loaded "root" user to test defaults for uid 0 + m_inspector.m_usergroup_manager.rm_user("", 0); + ASSERT_EQ(m_inspector.m_usergroup_manager.get_user("", 0), nullptr); + std::string path = "/home/file.txt"; int64_t fd = 3; @@ -74,6 +98,7 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_unexistent_user_entry) { ASSERT_EQ(get_field_as_string(evt, "user.uid"), "0"); ASSERT_EQ(get_field_as_string(evt, "user.name"), "root"); ASSERT_EQ(get_field_as_string(evt, "user.homedir"), "/root"); + ASSERT_EQ(get_field_as_string(evt, "user.shell"), ""); } TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry_without_metadata) { @@ -83,7 +108,7 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry_witho // Creating the entry in the user group manager will override // the one created by the inspector threadtable initial load. - // Since we set empty metadata, we don't expect any metadata in the output fields. + // Since we set "" metadatas, we don't expect any metadata in the output fields. m_inspector.m_usergroup_manager.add_user("", INIT_TID, 0, 0, "", "", ""); std::string path = "/home/file.txt"; @@ -99,4 +124,30 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry_witho ASSERT_EQ(get_field_as_string(evt, "user.uid"), "0"); ASSERT_EQ(get_field_as_string(evt, "user.name"), ""); ASSERT_EQ(get_field_as_string(evt, "user.homedir"), ""); + ASSERT_EQ(get_field_as_string(evt, "user.shell"), ""); +} + +TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_loaded_user_entry) { + add_default_init_thread(); + + open_inspector(); + + // Creating the entry in the user group manager will override + // the one created by the inspector threadtable initial load. + // Since we set **empty** metadata, we expect metadata to be loaded from the system. + m_inspector.m_usergroup_manager.add_user("", INIT_TID, 0, 0, {}, {}, {}); + + std::string path = "/home/file.txt"; + int64_t fd = 3; + + auto evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_OPEN_E, + fd, + path.c_str(), + (uint32_t)PPM_O_RDWR | PPM_O_CREAT, + (uint32_t)0); + ASSERT_EQ(get_field_as_string(evt, "user.uid"), "0"); + ASSERT_EQ(get_field_as_string(evt, "user.name"), "root"); + ASSERT_EQ(get_field_as_string(evt, "user.homedir"), "/root"); } diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index 9deffaad79..01d581f0c9 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -535,14 +535,14 @@ void sinsp_threadinfo::set_user(uint32_t uid) { scap_userinfo* user = m_inspector->m_usergroup_manager.get_user(m_container_id, uid); if(!user) { auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin(); - std::string name, home; // For uid 0 force set root related infos if(uid == 0) { - name = "root"; - home = "/root"; + m_inspector->m_usergroup_manager + .add_user(m_container_id, m_pid, uid, m_gid, "root", "/root", {}, notify); + } else { + m_inspector->m_usergroup_manager + .add_user(m_container_id, m_pid, uid, m_gid, {}, {}, {}, notify); } - m_inspector->m_usergroup_manager - .add_user(m_container_id, m_pid, uid, m_gid, name, home, {}, notify); } } @@ -551,12 +551,12 @@ void sinsp_threadinfo::set_group(uint32_t gid) { scap_groupinfo* group = m_inspector->m_usergroup_manager.get_group(m_container_id, gid); if(!group) { auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin(); - // For uid 0 force set root related infos - std::string name; + // For gid 0 force set root related info if(gid == 0) { - name = "root"; + m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, "root", notify); + } else { + m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, {}, notify); } - m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, name, notify); } } @@ -564,24 +564,20 @@ void sinsp_threadinfo::set_loginuid(uint32_t loginuid) { m_loginuid = loginuid; } -scap_userinfo* sinsp_threadinfo::get_user(uint32_t id) const { - auto user = m_inspector->m_usergroup_manager.get_user(m_container_id, id); +scap_userinfo* sinsp_threadinfo::get_user() const { + auto user = m_inspector->m_usergroup_manager.get_user(m_container_id, m_uid); if(user != nullptr) { return user; } static scap_userinfo usr{}; - usr.uid = id; + usr.uid = m_uid; usr.gid = m_gid; - strlcpy(usr.name, id == 0 ? "root" : "", sizeof(usr.name)); - strlcpy(usr.homedir, id == 0 ? "/root" : "", sizeof(usr.homedir)); + strlcpy(usr.name, m_uid == 0 ? "root" : "", sizeof(usr.name)); + strlcpy(usr.homedir, m_uid == 0 ? "/root" : "", sizeof(usr.homedir)); strlcpy(usr.shell, "", sizeof(usr.shell)); return &usr; } -scap_userinfo* sinsp_threadinfo::get_user() const { - return get_user(m_uid); -} - scap_groupinfo* sinsp_threadinfo::get_group() const { auto group = m_inspector->m_usergroup_manager.get_group(m_container_id, m_gid); if(group != nullptr) { @@ -594,7 +590,17 @@ scap_groupinfo* sinsp_threadinfo::get_group() const { } scap_userinfo* sinsp_threadinfo::get_loginuser() const { - return get_user(m_loginuid); + auto user = m_inspector->m_usergroup_manager.get_user(m_container_id, m_loginuid); + if(user != nullptr) { + return user; + } + static scap_userinfo usr{}; + usr.uid = m_loginuid; + usr.gid = m_gid; + strlcpy(usr.name, m_loginuid == 0 ? "root" : "", sizeof(usr.name)); + strlcpy(usr.homedir, m_loginuid == 0 ? "/root" : "", sizeof(usr.homedir)); + strlcpy(usr.shell, "", sizeof(usr.shell)); + return &usr; } void sinsp_threadinfo::set_args(const char* args, size_t len) { diff --git a/userspace/libsinsp/threadinfo.h b/userspace/libsinsp/threadinfo.h index 8998ff38e8..e03953fa05 100644 --- a/userspace/libsinsp/threadinfo.h +++ b/userspace/libsinsp/threadinfo.h @@ -608,8 +608,6 @@ class SINSP_PUBLIC sinsp_threadinfo : public libsinsp::state::table_entry { uint32_t& alen, std::string& rem) const; - scap_userinfo* get_user(uint32_t id) const; - // // Parameters that can't be accessed directly because they could be in the // parent thread info