[CWS] review changes

pkg/config/setup/system_probe_cws.go

@@ -118,9 +118,11 @@ func initCWSSystemProbeConfig(cfg pkgconfigmodel.Config) {
 	cfg.BindEnvAndSetDefault("runtime_security_config.hash_resolver.cache_size", 500)
 	cfg.BindEnvAndSetDefault("runtime_security_config.hash_resolver.replace", map[string]string{})
 
-	// CWS - SysCtl snapshot
-	cfg.BindEnvAndSetDefault("runtime_security_config.sysctl_snapshot.enabled", false)
-	cfg.BindEnvAndSetDefault("runtime_security_config.sysctl_snapshot.period", "1h")
+	// CWS - SysCtl
+	cfg.BindEnvAndSetDefault("runtime_security_config.sysctl.enabled", true)
+	cfg.BindEnvAndSetDefault("runtime_security_config.sysctl.snapshot.enabled", true)
+	cfg.BindEnvAndSetDefault("runtime_security_config.sysctl.snapshot.period", "1h")
+	cfg.BindEnvAndSetDefault("runtime_security_config.sysctl.snapshot.ignored_base_names", []string{"netdev_rss_key", "stable_secret"})
 
 	// CWS - UserSessions
 	cfg.BindEnvAndSetDefault("runtime_security_config.user_sessions.cache_size", 1024)

pkg/security/config/config.go

@@ -226,10 +226,14 @@ type RuntimeSecurityConfig struct {
 	// HashResolverReplace is used to apply specific hash to specific file path
 	HashResolverReplace map[string]string
 
+	// SysCtlEnabled defines if the sysctl event should be enabled
+	SysCtlEnabled bool
 	// SysCtlSnapshotEnabled defines if the sysctl snapshot feature should be enabled
 	SysCtlSnapshotEnabled bool
 	// SysCtlSnapshotPeriod defines at which time interval a new snapshot of sysctl parameters should be sent
 	SysCtlSnapshotPeriod time.Duration
+	// SysCtlSnapshotIgnoredBaseNames defines the list of basenaes that should be ignored from the snapshot
+	SysCtlSnapshotIgnoredBaseNames []string
 
 	// UserSessionsCacheSize defines the size of the User Sessions cache size
 	UserSessionsCacheSize int
@@ -419,8 +423,10 @@ func NewRuntimeSecurityConfig() (*RuntimeSecurityConfig, error) {
 		HashResolverReplace:        pkgconfigsetup.SystemProbe().GetStringMapString("runtime_security_config.hash_resolver.replace"),
 
 		// SysCtl config parameter
-		SysCtlSnapshotEnabled: pkgconfigsetup.SystemProbe().GetBool("runtime_security_config.sysctl_snapshot.enabled"),
-		SysCtlSnapshotPeriod:  pkgconfigsetup.SystemProbe().GetDuration("runtime_security_config.sysctl_snapshot.period"),
+		SysCtlEnabled:                  pkgconfigsetup.SystemProbe().GetBool("runtime_security_config.sysctl.enabled"),
+		SysCtlSnapshotEnabled:          pkgconfigsetup.SystemProbe().GetBool("runtime_security_config.sysctl.snapshot.enabled"),
+		SysCtlSnapshotPeriod:           pkgconfigsetup.SystemProbe().GetDuration("runtime_security_config.sysctl.snapshot.period"),
+		SysCtlSnapshotIgnoredBaseNames: pkgconfigsetup.SystemProbe().GetStringSlice("runtime_security_config.sysctl.snapshot.ignored_base_names"),
 
 		// security profiles
 		SecurityProfileEnabled:          pkgconfigsetup.SystemProbe().GetBool("runtime_security_config.security_profile.enabled"),

pkg/security/probe/probe.go

@@ -420,6 +420,11 @@ func (p *Probe) IsNetworkFlowMonitorEnabled() bool {
 	return p.IsNetworkEnabled() && p.Config.Probe.NetworkFlowMonitorEnabled
 }
 
+// IsSysctlEventEnabled returns whether the sysctl event is enabled
+func (p *Probe) IsSysctlEventEnabled() bool {
+	return p.Config.RuntimeSecurity.SysCtlEnabled
+}
+
 // IsActivityDumpEnabled returns whether activity dump is enabled
 func (p *Probe) IsActivityDumpEnabled() bool {
 	return p.Config.RuntimeSecurity.ActivityDumpEnabled

pkg/security/probe/probe_ebpf.go

@@ -150,15 +150,14 @@ type EBPFProbe struct {
 	supportsBPFSendSignal bool
 	processKiller         *ProcessKiller
 
-	isRuntimeDiscarded    bool
-	constantOffsets       map[string]uint64
-	runtimeCompiled       bool
-	useSyscallWrapper     bool
-	useFentry             bool
-	useRingBuffers        bool
-	useMmapableMaps       bool
-	cgroupSysctlSupported bool
-	cgroup2MountPath      string
+	isRuntimeDiscarded bool
+	constantOffsets    map[string]uint64
+	runtimeCompiled    bool
+	useSyscallWrapper  bool
+	useFentry          bool
+	useRingBuffers     bool
+	useMmapableMaps    bool
+	cgroup2MountPath   string
 
 	// On demand
 	onDemandManager     *OnDemandProbesManager
@@ -272,6 +271,10 @@ func (p *EBPFProbe) selectFentryMode() {
 	p.useFentry = true
 }
 
+func (p *EBPFProbe) isCgroupSysCtlNotSupported() bool {
+	return IsCgroupSysCtlNotSupported(p.kernelVersion, p.cgroup2MountPath)
+}
+
 func (p *EBPFProbe) isNetworkNotSupported() bool {
 	return IsNetworkNotSupported(p.kernelVersion)
 }
@@ -310,10 +313,15 @@ func (p *EBPFProbe) sanityChecks() error {
 	}
 
 	if p.config.Probe.NetworkFlowMonitorEnabled && p.isNetworkFlowMonitorNotSupported() {
-		seclog.Warnf("The network flow monitor feature of CWS requires a more recent kernel (at least 5.13) with support the bpf_for_each_elem map helper, setting event_monitoring_config.network.flow_monitor.enabled to false")
+		seclog.Warnf("The network flow monitor feature of CWS requires a more recent kernel (at least 5.13) with support for the bpf_for_each_elem map helper, setting event_monitoring_config.network.flow_monitor.enabled to false")
 		p.config.Probe.NetworkFlowMonitorEnabled = false
 	}
 
+	if p.config.RuntimeSecurity.SysCtlEnabled && p.isCgroupSysCtlNotSupported() {
+		seclog.Warnf("The sysctl tracking feature of CWS requires a more recent kernel with support for the cgroup/sysctl program type, setting runtime_security_config.sysctl.enabled to false")
+		p.config.RuntimeSecurity.SysCtlEnabled = false
+	}
+
 	return nil
 }
 
@@ -598,8 +606,10 @@ func (p *EBPFProbe) Start() error {
 	// start new tc classifier loop
 	go p.startSetupNewTCClassifierLoop()
 
-	// start sysctl snapshot loop
-	go p.startSysCtlSnapshotLoop()
+	if p.config.RuntimeSecurity.SysCtlSnapshotEnabled {
+		// start sysctl snapshot loop
+		go p.startSysCtlSnapshotLoop()
+	}
 
 	return p.eventStream.Start(&p.wg)
 }
@@ -1524,7 +1534,7 @@ func (p *EBPFProbe) validEventTypeForConfig(eventType string) bool {
 	case "network_flow_monitor":
 		return p.probe.IsNetworkFlowMonitorEnabled()
 	case "sysctl":
-		return p.cgroupSysctlSupported
+		return p.probe.IsSysctlEventEnabled()
 	}
 	return true
 }
@@ -1768,7 +1778,7 @@ func (p *EBPFProbe) startSysCtlSnapshotLoop() {
 			return
 		case <-ticker.C:
 			// create the sysctl snapshot
-			event, err := sysctl.NewSnapshotEvent()
+			event, err := sysctl.NewSnapshotEvent(p.config.RuntimeSecurity.SysCtlSnapshotIgnoredBaseNames)
 			if err != nil {
 				seclog.Errorf("sysctl snapshot failed: %v", err)
 				continue
@@ -2252,7 +2262,7 @@ func (p *EBPFProbe) initManagerOptionsExcludedFunctions() error {
 		p.managerOptions.AdditionalExcludedFunctionCollector = afBasedExcluder
 	}
 
-	if !p.cgroupSysctlSupported {
+	if !p.config.RuntimeSecurity.SysCtlEnabled {
 		p.managerOptions.ExcludedFunctions = append(p.managerOptions.ExcludedFunctions, probes.GetSysCtlProbeFunctionName())
 	}
 	return nil
@@ -2349,7 +2359,6 @@ func NewEBPFProbe(probe *Probe, config *config.Config, opts Opts) (*EBPFProbe, e
 	p.selectRingBuffersMode()
 	p.useMmapableMaps = p.kernelVersion.HaveMmapableMaps()
 	p.initCgroup2MountPath()
-	p.cgroupSysctlSupported = len(p.cgroup2MountPath) > 0 && p.kernelVersion.HasCgroupSysctlSupportWithRingbuf()
 
 	p.Manager = ebpf.NewRuntimeSecurityManager(p.useRingBuffers)
 

pkg/security/probe/probe_linux.go

@@ -70,6 +70,11 @@ func IsNetworkNotSupported(kv *kernel.Version) bool {
 	return kv.IsRH7Kernel() || kv.IsOracleUEKKernel()
 }
 
+// IsCgroupSysCtlNotSupported returns if the cgroup/sysctl program is supported
+func IsCgroupSysCtlNotSupported(kv *kernel.Version, cgroup2MountPath string) bool {
+	return len(cgroup2MountPath) > 0 && kv.HasCgroupSysctlSupportWithRingbuf()
+}
+
 // IsNetworkFlowMonitorNotSupported returns if the network flow monitor feature is supported
 func IsNetworkFlowMonitorNotSupported(kv *kernel.Version) bool {
 	return IsNetworkNotSupported(kv) || !kv.IsMapValuesToMapHelpersAllowed() || !kv.HasBPFForEachMapElemHelper()

pkg/security/probe/sysctl/snapshot.go

@@ -17,18 +17,16 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
+
+	"github.com/DataDog/datadog-agent/pkg/util/kernel"
 )
 
 var (
-	ignoredBaseNames = []string{
-		"netdev_rss_key",
-		"stable_secret",
-	}
 	redactedContent = "******"
 )
 
 // readFileContent reads a file and processes its content based on the given rules.
-func readFileContent(file string) (string, error) {
+func readFileContent(file string, ignoredBaseNames []string) (string, error) {
 	if slices.Contains(ignoredBaseNames, path.Base(file)) {
 		return redactedContent, nil
 	}
@@ -47,11 +45,11 @@ type SnapshotEvent struct {
 }
 
 // NewSnapshotEvent returns a new sysctl snapshot event
-func NewSnapshotEvent() (*SnapshotEvent, error) {
+func NewSnapshotEvent(ignoredBaseNames []string) (*SnapshotEvent, error) {
 	se := &SnapshotEvent{
 		Sysctl: NewSnapshot(),
 	}
-	if err := se.Sysctl.Snapshot(); err != nil {
+	if err := se.Sysctl.Snapshot(ignoredBaseNames); err != nil {
 		return nil, err
 	}
 	return se, nil
@@ -79,20 +77,20 @@ func NewSnapshot() Snapshot {
 }
 
 // Snapshot runs the snapshot by going through the filesystem
-func (s *Snapshot) Snapshot() error {
-	if err := s.snapshotProcSys(); err != nil {
+func (s *Snapshot) Snapshot(ignoredBaseNames []string) error {
+	if err := s.snapshotProcSys(ignoredBaseNames); err != nil {
 		return fmt.Errorf("couldn't snapshot /proc/sys: %w", err)
 	}
 
-	if err := s.snapshotSys(); err != nil {
+	if err := s.snapshotSys(ignoredBaseNames); err != nil {
 		return fmt.Errorf("coudln't snapshot /sys: %w", err)
 	}
 	return nil
 }
 
 // snapshotProcSys recursively reads files in /proc/sys and builds a nested JSON structure.
-func (s *Snapshot) snapshotProcSys() error {
-	return filepath.Walk("/proc/sys", func(file string, info fs.FileInfo, err error) error {
+func (s *Snapshot) snapshotProcSys(ignoredBaseNames []string) error {
+	return filepath.Walk(kernel.HostProc("/sys"), func(file string, info fs.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -104,16 +102,16 @@ func (s *Snapshot) snapshotProcSys() error {
 
 		// Skip if mode doesn't allow reading
 		mode := info.Mode()
-		if mode&0400 == 0 && mode&0040 == 0 && mode&0004 == 0 {
+		if mode&0444 == 0 {
 			return nil
 		}
 
-		relPath, err := filepath.Rel("/proc", file)
+		relPath, err := filepath.Rel(kernel.ProcFSRoot(), file)
 		if err != nil {
 			return err
 		}
 
-		value, err := readFileContent(file)
+		value, err := readFileContent(file, ignoredBaseNames)
 		if err != nil {
 			return nil // Skip files that can't be read
 		}
@@ -124,18 +122,17 @@ func (s *Snapshot) snapshotProcSys() error {
 }
 
 // snapshotSys reads security relevant files from the /sys filesystem
-func (s *Snapshot) snapshotSys() error {
-	lockdownValue, err := readFileContent("/sys/kernel/security/lockdown")
-	if err != nil {
-		return err
-	}
-	s.InsertSnapshotEntry(s.Sys, "kernel/security/lockdown", lockdownValue)
-
-	lsmValue, err := readFileContent("/sys/kernel/security/lsm")
-	if err != nil {
-		return err
+func (s *Snapshot) snapshotSys(ignoredBaseNames []string) error {
+	for _, systemControl := range []string{
+		"/kernel/security/lockdown",
+		"/kernel/security/lsm",
+	} {
+		value, err := readFileContent(kernel.HostSys(systemControl), ignoredBaseNames)
+		if err != nil {
+			return err
+		}
+		s.InsertSnapshotEntry(s.Sys, systemControl, value)
 	}
-	s.InsertSnapshotEntry(s.Sys, "kernel/security/lsm", lsmValue)
 	return nil
 }
 

pkg/security/probe/sysctl/snapshot_test.go

@@ -8,7 +8,7 @@
 package sysctl
 
 import (
-	"reflect"
+	"github.com/stretchr/testify/assert"
 	"testing"
 )
 
@@ -85,9 +85,7 @@ func TestSnapshotEvent_ToJSON(t *testing.T) {
 				t.Errorf("ToJSON() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			if !reflect.DeepEqual(string(got), tt.want) {
-				t.Errorf("ToJSON() got = %v, want %v", string(got), tt.want)
-			}
+			assert.JSONEqf(t, tt.want, string(got), "ToJSON() error")
 		})
 	}
 }
@@ -247,10 +245,7 @@ func TestSnapshot_InsertSnapshotEntry(t *testing.T) {
 				t.Errorf("InsertSnapshotEntry - ToJSON error: %v", err)
 				return
 			}
-			if !reflect.DeepEqual(string(got), tt.output) {
-				t.Errorf("InsertSnapshotEntry got = %v, want %v", string(got), tt.output)
-			}
-
+			assert.JSONEqf(t, string(got), tt.output, "InsertSnapshotEntry error")
 		})
 	}
 }

pkg/util/kernel/fs.go

@@ -65,6 +65,12 @@ func HostProc(combineWith ...string) string {
 	return filepath.Join(ProcFSRoot(), filepath.Join(combineWith...))
 }
 
+// HostSys returns the location of a host's sysfs. This can and will be
+// overridden when running inside a container.
+func HostSys(combineWith ...string) string {
+	return filepath.Join(SysFSRoot(), filepath.Join(combineWith...))
+}
+
 // RootNSPID returns the current PID from the root namespace
 var RootNSPID = funcs.Memoize(func() (int, error) {
 	pidPath := filepath.Join(ProcFSRoot(), "self")