Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: chore(userspace/libsinsp): move user group manager on container_id changed refresh to a RAII object #2194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+131 −107
Open

wip: chore(userspace/libsinsp): move user group manager on container_id changed refresh to a RAII object #2194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions userspace/libsinsp/dumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <libsinsp/dumper.h>
#include <libsinsp/plugin.h>
#include <libsinsp/plugin_manager.h>
#include <libsinsp/user.h>

sinsp_dumper::sinsp_dumper() {
m_dumper = NULL;
Expand Down Expand Up @@ -67,7 +68,7 @@

inspector->m_thread_manager->dump_threads_to_file(m_dumper);
inspector->m_container_manager.dump_containers(*this);
inspector->m_usergroup_manager.dump_users_groups(*this);
inspector->m_usergroup_manager->dump_users_groups(*this);

// ask registered ASYNC plugins for a dump of their state
for(auto& p : inspector->m_plugin_manager->plugins()) {
Expand All @@ -94,7 +95,7 @@

inspector->m_thread_manager->dump_threads_to_file(m_dumper);
inspector->m_container_manager.dump_containers(*this);
inspector->m_usergroup_manager.dump_users_groups(*this);
inspector->m_usergroup_manager->dump_users_groups(*this);

Check warning on line 98 in userspace/libsinsp/dumper.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/dumper.cpp#L98 

Added line #L98 was not covered by tests

// ask registered ASYNC plugins for a dump of their state
for(auto& p : inspector->m_plugin_manager->plugins()) {
Expand Down
6 changes: 4 additions & 2 deletions userspace/libsinsp/event.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

#include <libsinsp/sinsp.h>
#include <libsinsp/sinsp_int.h>
#include <libsinsp/user.h>
#include <libscap/strl.h>

#include <libscap/scap.h>
Expand Down Expand Up @@ -1325,7 +1326,7 @@
sinsp_threadinfo *tinfo = get_thread_info();
scap_userinfo *user_info = NULL;
if(tinfo) {
user_info = m_inspector->m_usergroup_manager.get_user(tinfo->m_container_id, val);
user_info = m_inspector->m_usergroup_manager->get_user(tinfo->m_container_id, val);
}
if(user_info != NULL) {
strcpy_sanitized(&m_resolved_paramstr_storage[0],
Expand Down Expand Up @@ -1354,7 +1355,8 @@
sinsp_threadinfo *tinfo = get_thread_info();
scap_groupinfo *group_info = NULL;
if(tinfo) {
group_info = m_inspector->m_usergroup_manager.get_group(tinfo->m_container_id, val);
group_info =

Check warning on line 1358 in userspace/libsinsp/event.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/event.cpp#L1358 

Added line #L1358 was not covered by tests
m_inspector->m_usergroup_manager->get_group(tinfo->m_container_id, val);
}
if(group_info != NULL) {
strcpy_sanitized(&m_resolved_paramstr_storage[0],
Expand Down
38 changes: 5 additions & 33 deletions userspace/libsinsp/parsers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ limitations under the License.
#include <libsinsp/plugin_manager.h>
#include <libsinsp/sinsp_observer.h>
#include <libsinsp/sinsp_int.h>
#include <libsinsp/user.h>

#if !defined(MINIMAL_BUILD) && !defined(__EMSCRIPTEN__)
#include <libsinsp/container_engine/docker/async_source.h>
Expand Down Expand Up @@ -1274,12 +1275,6 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) {
return;
}

/* Refresh user / group */
if(new_child->m_container_id.empty() == false) {
new_child->set_group(new_child->m_gid);
new_child->set_user(new_child->m_uid);
}

/* If there's a listener, invoke it */
if(m_inspector->get_observer()) {
m_inspector->get_observer()->on_clone(evt, new_child.get(), tid_collision);
Expand Down Expand Up @@ -1764,12 +1759,6 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) {
*/
evt->set_tinfo(new_child.get());

/* Refresh user / group */
if(new_child->m_container_id.empty() == false) {
new_child->set_group(new_child->m_gid);
new_child->set_user(new_child->m_uid);
}

//
// If there's a listener, invoke it
//
Expand Down Expand Up @@ -2239,15 +2228,6 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt) {
//
evt->get_tinfo()->compute_program_hash();

//
// Refresh user / group
// if we happen to change container id
//
if(container_id != evt->get_tinfo()->m_container_id) {
evt->get_tinfo()->set_group(evt->get_tinfo()->m_gid);
evt->get_tinfo()->set_user(evt->get_tinfo()->m_uid);
}

//
// If there's a listener, invoke it
//
Expand Down Expand Up @@ -4894,9 +4874,9 @@ void sinsp_parser::parse_user_evt(sinsp_evt *evt) {

if(evt->get_scap_evt()->type == PPME_USER_ADDED_E) {
m_inspector->m_usergroup_manager
.add_user(std::string(container_id), -1, uid, gid, name, home, shell);
->add_user(std::string(container_id), -1, uid, gid, name, home, shell);
} else {
m_inspector->m_usergroup_manager.rm_user(std::string(container_id), uid);
m_inspector->m_usergroup_manager->rm_user(std::string(container_id), uid);
}
}

Expand All @@ -4907,9 +4887,9 @@ void sinsp_parser::parse_group_evt(sinsp_evt *evt) {
std::string_view container_id = evt->get_param(2)->as<std::string_view>();

if(evt->get_scap_evt()->type == PPME_GROUP_ADDED_E) {
m_inspector->m_usergroup_manager.add_group(container_id.data(), -1, gid, name.data());
m_inspector->m_usergroup_manager->add_group(container_id.data(), -1, gid, name.data());
} else {
m_inspector->m_usergroup_manager.rm_group(container_id.data(), gid);
m_inspector->m_usergroup_manager->rm_group(container_id.data(), gid);
}
}

Expand Down Expand Up @@ -4992,14 +4972,6 @@ void sinsp_parser::parse_chroot_exit(sinsp_evt *evt) {
m_inspector->m_container_manager.resolve_container(
evt->get_tinfo(),
m_inspector->is_live() || m_inspector->is_syscall_plugin());
//
// Refresh user / group
// if we happen to change container id
//
if(container_id != evt->get_tinfo()->m_container_id) {
evt->get_tinfo()->set_group(evt->get_tinfo()->m_gid);
evt->get_tinfo()->set_user(evt->get_tinfo()->m_uid);
}
}
}

Expand Down
34 changes: 18 additions & 16 deletions userspace/libsinsp/sinsp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ limitations under the License.
#include <cstdlib>
#include <cstring>
#include <filesystem>
#include <user.h>

/**
* This is the maximum size assigned to the concurrent asynchronous event
Expand Down Expand Up @@ -148,7 +149,6 @@ sinsp::sinsp(bool with_metrics):
m_lastevent_ts(0),
m_host_root(scap_get_host_root()),
m_container_manager(this),
m_usergroup_manager(this),
m_async_events_queue(DEFAULT_ASYNC_EVENT_QUEUE_SIZE),
m_suppressed_comms(),
m_inited(false) {
Expand All @@ -163,6 +163,7 @@ sinsp::sinsp(bool with_metrics):
m_is_dumping = false;
m_parser = std::make_unique<sinsp_parser>(this);
m_thread_manager = std::make_unique<sinsp_thread_manager>(this);
m_usergroup_manager = std::make_unique<sinsp_usergroup_manager>(this);
m_max_fdtable_size = MAX_FD_TABLE_SIZE;
m_containers_purging_scan_time_ns = DEFAULT_INACTIVE_CONTAINER_SCAN_TIME_S * ONE_SECOND_IN_NS;
m_usergroups_purging_scan_time_ns = DEFAULT_DELETED_USERS_GROUPS_SCAN_TIME_S * ONE_SECOND_IN_NS;
Expand Down Expand Up @@ -359,7 +360,7 @@ void sinsp::init() {
}

void sinsp::set_import_users(bool import_users) {
m_usergroup_manager.m_import_users = import_users;
m_usergroup_manager->m_import_users = import_users;
}

/*=============================== OPEN METHODS ===============================*/
Expand All @@ -376,11 +377,7 @@ void sinsp::open_common(scap_open_args* oargs,
/* We need to save the actual mode and the engine used by the inspector. */
m_mode = mode;

oargs->import_users = m_usergroup_manager.m_import_users;
// We need to subscribe to container manager notifiers before
// scap starts scanning proc.
m_usergroup_manager.subscribe_container_mgr();

oargs->import_users = m_usergroup_manager->m_import_users;
oargs->log_fn = &sinsp_scap_log_fn;
oargs->proc_scan_timeout_ms = m_proc_scan_timeout_ms;
oargs->proc_scan_log_interval_ms = m_proc_scan_log_interval_ms;
Expand Down Expand Up @@ -990,17 +987,17 @@ void sinsp::import_user_list() {

if(ul) {
for(j = 0; j < ul->nusers; j++) {
m_usergroup_manager.add_user("",
-1,
ul->users[j].uid,
ul->users[j].gid,
ul->users[j].name,
ul->users[j].homedir,
ul->users[j].shell);
m_usergroup_manager->add_user("",
-1,
ul->users[j].uid,
ul->users[j].gid,
ul->users[j].name,
ul->users[j].homedir,
ul->users[j].shell);
}

for(j = 0; j < ul->ngroups; j++) {
m_usergroup_manager.add_group("", -1, ul->groups[j].gid, ul->groups[j].name);
m_usergroup_manager->add_group("", -1, ul->groups[j].gid, ul->groups[j].name);
}
}
}
Expand Down Expand Up @@ -1267,7 +1264,7 @@ int32_t sinsp::next(sinsp_evt** puevt) {
}

if(m_auto_usergroups_purging && !is_offline()) {
m_usergroup_manager.clear_host_users_groups();
m_usergroup_manager->clear_host_users_groups();
}

//
Expand Down Expand Up @@ -1297,6 +1294,11 @@ int32_t sinsp::next(sinsp_evt** puevt) {
{
// Object that uses RAII to enable event filtered out flag
sinsp_evt_filter evt_filter(evt);
// Object that uses RAII to automatically update user/group associated with a threadinfo
// upon threadinfo's container_id changes.
// Since the threadinfo state might get changed from a plugin parser,
// evaluate this one after all parsers get run.
user_group_updater usr_grp_updater(evt);

if(!evt->is_filtered_out()) {
//
Expand Down
4 changes: 2 additions & 2 deletions userspace/libsinsp/sinsp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ limitations under the License.
#include <libsinsp/metrics_collector.h>
#include <libsinsp/threadinfo.h>
#include <libsinsp/tuples.h>
#include <libsinsp/user.h>
#include <libsinsp/utils.h>

#include <list>
Expand All @@ -91,6 +90,7 @@ class sinsp_filter;
class sinsp_plugin;
class sinsp_plugin_manager;
class sinsp_observer;
class sinsp_usergroup_manager;

/*!
\brief The user agent string to use for any libsinsp connection, can be changed at compile time
Expand Down Expand Up @@ -1062,7 +1062,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source {

sinsp_container_manager m_container_manager;

sinsp_usergroup_manager m_usergroup_manager;
std::unique_ptr<sinsp_usergroup_manager> m_usergroup_manager;

//
// True if the command line argument is set to show container information
Expand Down
6 changes: 3 additions & 3 deletions userspace/libsinsp/test/filterchecks/evt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,16 +195,16 @@

// 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"));
->add_user(container_id, 0, user_id, 6, "test", "/test", "/bin/test"));

Check warning on line 198 in userspace/libsinsp/test/filterchecks/evt.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/filterchecks/evt.cpp#L198 

Added line #L198 was not covered by tests

// 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));
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>");
Expand Down
16 changes: 8 additions & 8 deletions userspace/libsinsp/test/filterchecks/user.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", "/foo", "/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;
Expand All @@ -49,7 +49,7 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry) {
ASSERT_EQ(get_field_as_string(evt, "user.loginname"), "root");

// Now remove the user
m_inspector.m_usergroup_manager.rm_user("", 1000);
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"), "<NA>");
Expand All @@ -59,7 +59,7 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_existent_user_entry) {
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");
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");
Expand All @@ -76,11 +76,11 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_default_user_entry) {

// 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);
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);
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;
Expand Down Expand Up @@ -109,7 +109,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 "" metadatas, we don't expect any metadata in the output fields.
m_inspector.m_usergroup_manager.add_user("", INIT_TID, 0, 0, "", "", "");
m_inspector.m_usergroup_manager->add_user("", INIT_TID, 0, 0, "", "", "");

std::string path = "/home/file.txt";
int64_t fd = 3;
Expand All @@ -135,7 +135,7 @@ TEST_F(sinsp_with_test_input, USER_FILTER_extract_from_loaded_user_entry) {
// 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, {}, {}, {});
m_inspector.m_usergroup_manager->add_user("", INIT_TID, 0, 0, {}, {}, {});

std::string path = "/home/file.txt";
int64_t fd = 3;
Expand Down
1 change: 1 addition & 0 deletions userspace/libsinsp/test/sinsp_with_test_input.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ limitations under the License.

#include <libscap/scap.h>
#include <libsinsp/sinsp.h>
#include <libsinsp/user.h>
#include <libsinsp/filterchecks.h>
#include <libscap/strl.h>
#include <libsinsp_test_var.h>
Expand Down
Loading
Loading