Allow to ignore process arguments (#1898)

Pass CollectorConfig to the ProcessSignalHandler and
ProcessSignalFormatter. It's needed for the future work, involving
conditional logic in the process handler.

Use the configuration above to allow to not collect process arguments.

collector/lib/CollectorConfig.cpp

@@ -83,6 +83,7 @@ PathEnvVar tls_client_key_path("ROX_COLLECTOR_TLS_CLIENT_KEY");
 
 PathEnvVar config_file("ROX_COLLECTOR_CONFIG_PATH", "/etc/stackrox/runtime_config.yaml");
 
+BoolEnvVar disable_process_arguments("ROX_COLLECTOR_NO_PROCESS_ARGUMENTS", false);
 }  // namespace
 
 constexpr bool CollectorConfig::kTurnOffScrape;
@@ -113,6 +114,7 @@ void CollectorConfig::InitCollectorConfig(CollectorArgs* args) {
   use_podman_ce_ = use_podman_ce.value();
   enable_introspection_ = enable_introspection.value();
   track_send_recv_ = track_send_recv.value();
+  disable_process_arguments_ = disable_process_arguments.value();
 
   for (const auto& syscall : kSyscalls) {
     syscalls_.emplace_back(syscall);

collector/lib/CollectorConfig.h

@@ -132,6 +132,7 @@ class CollectorConfig {
   unsigned int GetSinspThreadCacheSize() const { return sinsp_thread_cache_size_; }
   void Start();
   void Stop();
+  bool DisableProcessArguments() const { return disable_process_arguments_; }
 
   static std::pair<option::ArgStatus, std::string> CheckConfiguration(const char* config, Json::Value* root);
 
@@ -184,6 +185,8 @@ class CollectorConfig {
   // URL to the GRPC server
   std::optional<std::string> grpc_server_;
 
+  bool disable_process_arguments_ = false;
+
   // One ring buffer will be initialized for this many CPUs
   unsigned int sinsp_cpu_per_buffer_ = 0;
   // Size of one ring buffer, in bytes.

collector/lib/ProcessSignalFormatter.cpp

@@ -56,7 +56,12 @@ std::string extract_proc_args(sinsp_threadinfo* tinfo) {
 
 }  // namespace
 
-ProcessSignalFormatter::ProcessSignalFormatter(sinsp* inspector) : event_names_(EventNames::GetInstance()), event_extractor_(std::make_unique<system_inspector::EventExtractor>()), container_metadata_(inspector) {
+ProcessSignalFormatter::ProcessSignalFormatter(
+    sinsp* inspector,
+    const CollectorConfig& config) : event_names_(EventNames::GetInstance()),
+                                     event_extractor_(std::make_unique<system_inspector::EventExtractor>()),
+                                     container_metadata_(inspector),
+                                     config_(config) {
   event_extractor_->Init(inspector);
 }
 
@@ -143,11 +148,13 @@ ProcessSignal* ProcessSignalFormatter::CreateProcessSignal(sinsp_evt* event) {
     signal->set_exec_file_path(name_sanitized ? *name_sanitized : *name);
   }
 
-  // set process arguments
-  if (const char* args = event_extractor_->get_proc_args(event)) {
-    std::string args_str = args;
-    auto args_sanitized = SanitizedUTF8(args_str);
-    signal->set_args(args_sanitized ? *args_sanitized : args_str);
+  // set process arguments, if not explicitely disabled
+  if (!config_.DisableProcessArguments()) {
+    if (const char* args = event_extractor_->get_proc_args(event)) {
+      std::string args_str = args;
+      auto args_sanitized = SanitizedUTF8(args_str);
+      signal->set_args(args_sanitized ? *args_sanitized : args_str);
+    }
   }
 
   // set pid

collector/lib/ProcessSignalFormatter.h

@@ -3,10 +3,13 @@
 
 #include <memory>
 
+#include <gtest/gtest_prod.h>
+
 #include "api/v1/signal.pb.h"
 #include "internalapi/sensor/signal_iservice.pb.h"
 #include "storage/process_indicator.pb.h"
 
+#include "CollectorConfig.h"
 #include "CollectorStats.h"
 #include "ContainerMetadata.h"
 #include "EventNames.h"
@@ -25,7 +28,7 @@ namespace collector {
 
 class ProcessSignalFormatter : public ProtoSignalFormatter<sensor::SignalStreamMessage> {
  public:
-  ProcessSignalFormatter(sinsp* inspector);
+  ProcessSignalFormatter(sinsp* inspector, const CollectorConfig& config);
   ~ProcessSignalFormatter();
 
   using Signal = v1::Signal;
@@ -38,8 +41,11 @@ class ProcessSignalFormatter : public ProtoSignalFormatter<sensor::SignalStreamM
   void GetProcessLineage(sinsp_threadinfo* tinfo, std::vector<LineageInfo>& lineage);
 
  private:
-  Signal* CreateSignal(sinsp_evt* event);
+  FRIEND_TEST(ProcessSignalFormatterTest, NoProcessArguments);
+  FRIEND_TEST(ProcessSignalFormatterTest, ProcessArguments);
+
   ProcessSignal* CreateProcessSignal(sinsp_evt* event);
+  Signal* CreateSignal(sinsp_evt* event);
   bool ValidateProcessDetails(const sinsp_threadinfo* tinfo);
   bool ValidateProcessDetails(sinsp_evt* event);
   std::string ProcessDetails(sinsp_evt* event);
@@ -52,6 +58,8 @@ class ProcessSignalFormatter : public ProtoSignalFormatter<sensor::SignalStreamM
   const EventNames& event_names_;
   std::unique_ptr<system_inspector::EventExtractor> event_extractor_;
   ContainerMetadata container_metadata_;
+
+  const CollectorConfig& config_;
 };
 
 }  // namespace collector

collector/lib/ProcessSignalHandler.h

@@ -5,6 +5,7 @@
 
 #include <grpcpp/channel.h>
 
+#include "CollectorConfig.h"
 #include "ProcessSignalFormatter.h"
 #include "RateLimit.h"
 #include "SignalHandler.h"
@@ -19,8 +20,15 @@ namespace collector {
 
 class ProcessSignalHandler : public SignalHandler {
  public:
-  ProcessSignalHandler(sinsp* inspector, ISignalServiceClient* client, system_inspector::Stats* stats)
-      : client_(client), formatter_(inspector), stats_(stats) {}
+  ProcessSignalHandler(
+      sinsp* inspector,
+      ISignalServiceClient* client,
+      system_inspector::Stats* stats,
+      const CollectorConfig& config)
+      : client_(client),
+        formatter_(inspector, config),
+        stats_(stats),
+        config_(config) {}
 
   bool Start() override;
   bool Stop() override;
@@ -34,6 +42,8 @@ class ProcessSignalHandler : public SignalHandler {
   ProcessSignalFormatter formatter_;
   system_inspector::Stats* stats_;
   RateLimitCache rate_limiter_;
+
+  const CollectorConfig& config_;
 };
 
 }  // namespace collector

collector/lib/system-inspector/Service.cpp

@@ -58,7 +58,8 @@ void Service::Init(const CollectorConfig& config, std::shared_ptr<ConnectionTrac
   }
   AddSignalHandler(MakeUnique<ProcessSignalHandler>(inspector_.get(),
                                                     signal_client_.get(),
-                                                    &userspace_stats_));
+                                                    &userspace_stats_,
+                                                    config));
 
   if (signal_handlers_.size() == 2) {
     // self-check handlers do not count towards this check, because they

collector/test/ProcessSignalFormatterTest.cpp

@@ -10,16 +10,24 @@
 
 namespace collector {
 
-using ProcessSignal = ProcessSignalFormatter::ProcessSignal;
 using LineageInfo = ProcessSignalFormatter::LineageInfo;
+using namespace testing;
 
-namespace {
+class MockCollectorConfig : public CollectorConfig {
+ public:
+  MockCollectorConfig() = default;
+
+  void SetDisableProcessArguments(bool value) {
+    disable_process_arguments_ = value;
+  }
+};
 
 TEST(ProcessSignalFormatterTest, NoProcessTest) {
   sinsp* inspector = NULL;
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector);
+  ProcessSignalFormatter processSignalFormatter(inspector, config);
 
   sinsp_threadinfo* tinfo = NULL;
   std::vector<LineageInfo> lineage;
@@ -42,8 +50,9 @@ TEST(ProcessSignalFormatterTest, NoProcessTest) {
 TEST(ProcessSignalFormatterTest, ProcessWithoutParentTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 0;
@@ -76,8 +85,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithoutParentTest) {
 TEST(ProcessSignalFormatterTest, ProcessWithParentTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -120,8 +130,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithParentTest) {
 TEST(ProcessSignalFormatterTest, ProcessWithParentWithPid0Test) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 0;
@@ -159,8 +170,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithParentWithPid0Test) {
 TEST(ProcessSignalFormatterTest, ProcessWithParentWithSameNameTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -203,8 +215,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithParentWithSameNameTest) {
 TEST(ProcessSignalFormatterTest, ProcessWithTwoParentsTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -261,8 +274,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithTwoParentsTest) {
 TEST(ProcessSignalFormatterTest, ProcessWithTwoParentsWithTheSameNameTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -316,8 +330,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithTwoParentsWithTheSameNameTest) {
 TEST(ProcessSignalFormatterTest, ProcessCollapseParentChildWithSameNameTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -380,8 +395,9 @@ TEST(ProcessSignalFormatterTest, ProcessCollapseParentChildWithSameNameTest) {
 TEST(ProcessSignalFormatterTest, ProcessCollapseParentChildWithSameName2Test) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -447,8 +463,9 @@ TEST(ProcessSignalFormatterTest, ProcessCollapseParentChildWithSameName2Test) {
 TEST(ProcessSignalFormatterTest, ProcessWithUnrelatedProcessTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -514,8 +531,9 @@ TEST(ProcessSignalFormatterTest, ProcessWithUnrelatedProcessTest) {
 TEST(ProcessSignalFormatterTest, CountTwoCounterCallsTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 1;
@@ -561,8 +579,9 @@ TEST(ProcessSignalFormatterTest, CountTwoCounterCallsTest) {
 TEST(ProcessSignalFormatterTest, Rox3377ProcessLineageWithNoVPidTest) {
   std::unique_ptr<sinsp> inspector(new sinsp());
   CollectorStats& collector_stats = CollectorStats::GetOrCreate();
+  CollectorConfig config;
 
-  ProcessSignalFormatter processSignalFormatter(inspector.get());
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
 
   auto tinfo = inspector->build_threadinfo();
   tinfo->m_pid = 3;
@@ -616,6 +635,63 @@ TEST(ProcessSignalFormatterTest, Rox3377ProcessLineageWithNoVPidTest) {
   CollectorStats::Reset();
 }
 
-}  // namespace
+TEST(ProcessSignalFormatterTest, ProcessArguments) {
+  std::unique_ptr<sinsp> inspector(new sinsp());
+  MockCollectorConfig config;
+
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
+
+  auto tinfo = inspector->build_threadinfo();
+  tinfo->m_pid = 3;
+  tinfo->m_tid = 3;
+  tinfo->m_ptid = -1;
+  tinfo->m_vpid = 0;
+  tinfo->m_user.set_uid(42);
+  tinfo->m_container_id = "";
+  tinfo->m_exepath = "qwerty";
+
+  std::vector<std::string> args = {std::string("args")};
+  tinfo->set_args(args);
+
+  std::unique_ptr<sinsp_evt> evt(new sinsp_evt());
+  std::unique_ptr<scap_evt> s_evt(new scap_evt());
+
+  s_evt->type = PPME_SYSCALL_EXECVE_19_X;
+  evt.get()->set_tinfo(tinfo.get());
+  evt.get()->set_scap_evt(s_evt.get());
+
+  auto signal = processSignalFormatter.CreateProcessSignal(evt.get());
+  EXPECT_FALSE(signal->args().empty());
+}
+
+TEST(ProcessSignalFormatterTest, NoProcessArguments) {
+  std::unique_ptr<sinsp> inspector(new sinsp());
+  MockCollectorConfig config;
+
+  config.SetDisableProcessArguments(true);
+  ProcessSignalFormatter processSignalFormatter(inspector.get(), config);
+
+  auto tinfo = inspector->build_threadinfo();
+  tinfo->m_pid = 3;
+  tinfo->m_tid = 3;
+  tinfo->m_ptid = -1;
+  tinfo->m_vpid = 0;
+  tinfo->m_user.set_uid(42);
+  tinfo->m_container_id = "";
+  tinfo->m_exepath = "qwerty";
+
+  std::vector<std::string> args = {std::string("args")};
+  tinfo->set_args(args);
+
+  std::unique_ptr<sinsp_evt> evt(new sinsp_evt());
+  std::unique_ptr<scap_evt> s_evt(new scap_evt());
+
+  s_evt->type = PPME_SYSCALL_EXECVE_19_X;
+  evt.get()->set_tinfo(tinfo.get());
+  evt.get()->set_scap_evt(s_evt.get());
+
+  auto signal = processSignalFormatter.CreateProcessSignal(evt.get());
+  EXPECT_TRUE(signal->args().empty());
+}
 
 }  // namespace collector