[CWS] review changes

pkg/config/setup/system_probe_cws.go

@@ -118,9 +118,10 @@ 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")
 
 	// CWS - UserSessions
 	cfg.BindEnvAndSetDefault("runtime_security_config.user_sessions.cache_size", 1024)

pkg/security/config/config.go

@@ -226,6 +226,8 @@ 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
@@ -419,8 +421,9 @@ 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"),
 
 		// 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,15 @@ 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
+	cgroupSysctlEnabled bool
+	cgroup2MountPath    string
 
 	// On demand
 	onDemandManager     *OnDemandProbesManager
@@ -272,6 +272,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 +314,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 +607,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)
 }
@@ -1518,7 +1529,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
 }
@@ -2246,7 +2257,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
@@ -2343,7 +2354,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

@@ -11,6 +11,7 @@ package sysctl
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/DataDog/datadog-agent/pkg/util/kernel"
 	"io/fs"
 	"os"
 	"path"
@@ -92,7 +93,7 @@ func (s *Snapshot) Snapshot() error {
 
 // 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 {
+	return filepath.Walk(kernel.HostProc("/sys"), func(file string, info fs.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -104,11 +105,11 @@ 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
 		}
@@ -125,17 +126,16 @@ 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
+	for _, systemControl := range []string{
+		"/kernel/security/lockdown",
+		"/kernel/security/lsm",
+	} {
+		value, err := readFileContent(kernel.HostSys(systemControl))
+		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")