dd-trace-go icon indicating copy to clipboard operation
dd-trace-go copied to clipboard

appsec: upgrade go-libddwaf/v3

Open Hellzy opened this issue 1 year ago • 1 comments

What does this PR do?

JIRA: APPSEC-52726

Upgrades the go-libddwaf dependency to v3. In v3, actions are now returned as a map[string]any mapping action types to their parameters. Parameters, while technicaly described as a map[string]any, are actually map[string]string (i.e all param values are provided as strings).

Changes

  • New action package that defines action parameter types and provides utilities to translate waf result action values to actual go values
  • Remove former action caching system which would embed actions in waf handles.
  • Remove waf handle wrapping which is not necessary anymore
  • Don't parse actions out of the ruleset, which is not necessary anymore since we don't cache them
  • After a WAF run, we now:
    • parse actions returned through WAF results (new)
    • generate the actions (new)
    • propagate actions through dyngo and execute them

Motivation

Reviewer's Checklist

  • [ ] Changed code has unit tests for its functionality at or near 100% coverage.
  • [ ] System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • [ ] There is a benchmark for any new code, or changes to existing code.
  • [ ] If this interacts with the agent in a new way, a system test has been added.
  • [ ] Add an appropriate team label so this PR gets put in the right place for the release notes.
  • [ ] Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

Hellzy avatar Apr 25 '24 15:04 Hellzy

Benchmarks

Benchmark execution time: 2024-05-21 14:10:46

Comparing candidate commit 50141700d97c2da2505670fc2e345ebeb125ed87 in PR branch francois.mazeau/go-libddwaf-update with baseline commit d823db50b28eccb1b354616df2887266da3001ad in branch main.

Found 0 performance improvements and 6 performance regressions! Performance is the same for 38 metrics, 0 unstable metrics.

scenario:BenchmarkPartialFlushing/Disabled-24

  • 🟥 execution_time [+10.554ms; +13.609ms] or [+3.765%; +4.855%]

scenario:BenchmarkPartialFlushing/Enabled-24

  • 🟥 execution_time [+9.236ms; +12.138ms] or [+3.335%; +4.383%]

scenario:BenchmarkSingleSpanRetention/no-rules-24

  • 🟥 execution_time [+10.040µs; +10.704µs] or [+4.186%; +4.463%]

scenario:BenchmarkSingleSpanRetention/with-rules/match-all-24

  • 🟥 execution_time [+9.195µs; +11.184µs] or [+3.789%; +4.608%]

scenario:BenchmarkSingleSpanRetention/with-rules/match-half-24

  • 🟥 execution_time [+9.402µs; +11.052µs] or [+3.868%; +4.546%]

scenario:BenchmarkStartSpan-24

  • 🟥 execution_time [+69.499ns; +117.101ns] or [+3.040%; +5.122%]

pr-commenter[bot] avatar May 06 '24 16:05 pr-commenter[bot]