Skip to content

Commit 5156a99

Browse files
authored
[CWS] Resolve file path before performing a hash action (#46012)
### What does this PR do? Force a file path resolution when performing a hash action. ### Motivation Otherwise the path is empty for rules like the following and the hash computation just fails: ```yaml id: hash_open expression: open.flags&O_CREAT == O_CREAT actions: - hash: field: open.file ``` ### Describe how you validated your changes ### Additional Notes Co-authored-by: yoann.ghigoff <yoann.ghigoff@datadoghq.com>
1 parent 8b2527f commit 5156a99

4 files changed

Lines changed: 97 additions & 13 deletions

File tree

pkg/security/probe/file_hasher.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/DataDog/datadog-agent/pkg/security/resolvers/hash"
1818
"github.com/DataDog/datadog-agent/pkg/security/secl/model"
1919
"github.com/DataDog/datadog-agent/pkg/security/secl/rules"
20-
"github.com/DataDog/datadog-agent/pkg/security/seclog"
2120
"github.com/DataDog/datadog-agent/pkg/security/utils"
2221
)
2322

@@ -93,19 +92,11 @@ func (p *FileHasher) HandleProcessExited(event *model.Event) {
9392
}
9493

9594
// HashAndReport hash and report, returns true if the hash computation is supported for the given event
96-
func (p *FileHasher) HashAndReport(rule *rules.Rule, action *rules.HashDefinition, ev *model.Event) bool {
97-
eventType := ev.GetEventType()
98-
95+
func (p *FileHasher) HashAndReport(rule *rules.Rule, action *rules.HashDefinition, ev *model.Event, fileEvent *model.FileEvent) bool {
9996
if !p.cfg.RuntimeSecurity.HashResolverEnabled {
10097
return false
10198
}
10299

103-
fileEvent, err := ev.GetFileField(action.Field)
104-
if err != nil {
105-
seclog.Errorf("failed to get file field %s: %v", action.Field, err)
106-
return false
107-
}
108-
109100
if ev.ProcessContext.Pid == utils.Getpid() {
110101
return false
111102
}
@@ -124,7 +115,7 @@ func (p *FileHasher) HashAndReport(rule *rules.Rule, action *rules.HashDefinitio
124115
maxFileSize: action.MaxFileSize,
125116
seenAt: ev.ResolveEventTime(),
126117
fileEvent: *fileEvent,
127-
eventType: eventType,
118+
eventType: ev.GetEventType(),
128119
}
129120
ev.ActionReports = append(ev.ActionReports, report)
130121
p.pendingReports = append(p.pendingReports, report)

pkg/security/probe/probe_ebpf.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3420,7 +3420,17 @@ func (p *EBPFProbe) HandleActions(ctx *eval.Context, rule *rules.Rule) {
34203420
p.probe.onRuleActionPerformed(rule, action.Def)
34213421
}
34223422
case action.Def.Hash != nil:
3423-
if p.fileHasher.HashAndReport(rule, action.Def.Hash, ev) {
3423+
fileEvent, err := ev.GetFileField(action.Def.Hash.Field)
3424+
if err != nil {
3425+
seclog.Errorf("failed to get file field %s: %v", action.Def.Hash.Field, err)
3426+
continue
3427+
}
3428+
3429+
if p.fieldHandlers.ResolveFilePath(ev, fileEvent) == "" {
3430+
continue
3431+
}
3432+
3433+
if p.fileHasher.HashAndReport(rule, action.Def.Hash, ev, fileEvent) {
34243434
p.probe.onRuleActionPerformed(rule, action.Def)
34253435
}
34263436
case action.Def.NetworkFilter != nil:

pkg/security/probe/probe_ebpfless.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,17 @@ func (p *EBPFLessProbe) HandleActions(ctx *eval.Context, rule *rules.Rule) {
667667
p.probe.onRuleActionPerformed(rule, action.Def)
668668
}
669669
case action.Def.Hash != nil:
670-
if p.fileHasher.HashAndReport(rule, action.Def.Hash, ev) {
670+
fileEvent, err := ev.GetFileField(action.Def.Hash.Field)
671+
if err != nil {
672+
seclog.Errorf("failed to get file field %s: %v", action.Def.Hash.Field, err)
673+
continue
674+
}
675+
676+
if p.fieldHandlers.ResolveFilePath(ev, fileEvent) == "" {
677+
continue
678+
}
679+
680+
if p.fileHasher.HashAndReport(rule, action.Def.Hash, ev, fileEvent) {
671681
p.probe.onRuleActionPerformed(rule, action.Def)
672682
}
673683
}

pkg/security/tests/action_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,79 @@ func TestActionHash(t *testing.T) {
785785
}, retry.Delay(500*time.Millisecond), retry.Attempts(30), retry.DelayType(retry.FixedDelay))
786786
assert.NoError(t, err)
787787
})
788+
789+
t.Run("open-force-path", func(t *testing.T) {
790+
// test that we correctly force a path resolution when we run the hash action
791+
newRuleDefs := []*rules.RuleDefinition{
792+
{
793+
ID: "hash_action_open_no_path",
794+
Expression: `open.flags&O_CREAT == O_CREAT && process.file.name == "syscall_tester"`,
795+
Actions: []*rules.ActionDefinition{
796+
{
797+
Hash: &rules.HashDefinition{
798+
Field: "open.file",
799+
},
800+
},
801+
},
802+
},
803+
}
804+
805+
// Set the new policy and reload (without closing/restarting the module)
806+
// On reload, exec events are replayed for running processes, so the kill rule should trigger
807+
if err := setTestPolicy(commonCfgDir, nil, newRuleDefs); err != nil {
808+
t.Fatalf("failed to set new policy: %v", err)
809+
}
810+
811+
err := test.reloadPolicies()
812+
if err != nil {
813+
t.Fatalf("failed to reload policies: %v", err)
814+
}
815+
816+
test.msgSender.flush()
817+
test.WaitSignalFromRule(t, func() error {
818+
go func() {
819+
timeoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
820+
defer cancel()
821+
822+
if err := runSyscallTesterFunc(
823+
timeoutCtx, t, syscallTester,
824+
"slow-write", "2", testFile, "aaa",
825+
); err != nil {
826+
t.Error(err)
827+
}
828+
829+
done <- true
830+
}()
831+
return nil
832+
}, func(_ *model.Event, rule *rules.Rule) {
833+
assertTriggeredRule(t, rule, "hash_action_open_no_path")
834+
}, "hash_action_open_no_path")
835+
836+
err = retry.Do(func() error {
837+
msg := test.msgSender.getMsg("hash_action_open_no_path")
838+
if msg == nil {
839+
return errors.New("not found")
840+
}
841+
validateMessageSchema(t, string(msg.Data))
842+
843+
jsonPathValidation(test, msg.Data, func(_ *testModule, obj interface{}) {
844+
if el, err := jsonpath.JsonPathLookup(obj, `$.agent.rule_actions[?(@.state == 'Done')]`); err != nil || el == nil || len(el.([]interface{})) == 0 {
845+
t.Errorf("element not found %s => %v", string(msg.Data), err)
846+
}
847+
if el, err := jsonpath.JsonPathLookup(obj, `$.agent.rule_actions[?(@.trigger == 'process_exit')]`); err != nil || el == nil || len(el.([]interface{})) == 0 {
848+
t.Errorf("element not found %s => %v", string(msg.Data), err)
849+
}
850+
if el, err := jsonpath.JsonPathLookup(obj, `$.file.hashes`); err != nil || el == nil || len(el.([]interface{})) == 0 {
851+
t.Errorf("element not found %s => %v", string(msg.Data), err)
852+
}
853+
})
854+
855+
return nil
856+
}, retry.Delay(500*time.Millisecond), retry.Attempts(30), retry.DelayType(retry.FixedDelay))
857+
assert.NoError(t, err)
858+
859+
<-done
860+
})
788861
}
789862

790863
func TestActionKillWithSignature(t *testing.T) {

0 commit comments

Comments
 (0)