contrib/google.golang.org/grpc: improve the memory efficiency of threats detection for grpc #2338

RomainMuller · 2023-11-07T15:18:11Z

What does this PR do?

Upgrades github.com/DataDog/go-libddwaf to the first commit that includes libddwaf@1.5, which is capable of ephemeral addresses, allowing workloads such as gRPC streaming and GraphQL to evaluate certain rules multiple times without having to reset the entire WAF context each time.

Motivation

Improved ASM capabilities for protocols that allow for multiple sub-requests to be composed into a single top-level request (such as gRPC streaming, GraphQL, ...)

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
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.

For Datadog employees:

If this PR touches code that handles credentials of any kind, such as Datadog API keys, I've requested a review from @DataDog/security-design-and-guidance.
This PR doesn't touch any of that.

pr-commenter · 2023-11-07T15:32:43Z

Benchmarks

Benchmark execution time: 2023-11-14 13:41:59

Comparing candidate commit dbaa2fc in PR branch romain.marcadier/waf-upgrade-ephemeral/APPSEC-12062 with baseline commit 403e56d in branch main.

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

contrib/google.golang.org/grpc/appsec_test.go

go.mod

internal/appsec/remoteconfig.go

internal/appsec/waf.go

internal/appsec/waf_unit_test.go

internal/appsec/waf.go

github-actions · 2023-11-14T13:24:50Z

internal/appsec/waf.go

@@ -59,6 +59,11 @@ func (a *appsec) swapWAF(rules rulesFragment) (err error) {
 		}
 	}()

+	wafDiags := newHandle.Diagnostics()
+	if err := wafDiags.TopLevelError(); err != nil {


🚫 [golangci] _{reported by reviewdog 🐶}
wafDiags.TopLevelError undefined (type waf.Diagnostics has no field or method TopLevelError)) (typecheck)

github-actions · 2023-11-14T13:24:50Z

internal/appsec/waf.go

@@ -59,6 +59,11 @@
 		}
 	}()

+	wafDiags := newHandle.Diagnostics()
+	if err := wafDiags.TopLevelError(); err != nil {


🚫 [golangci] _{reported by reviewdog 🐶}
wafDiags.TopLevelError undefined (type waf.Diagnostics has no field or method TopLevelError)) (typecheck)

github-actions · 2023-11-14T13:24:50Z

internal/appsec/waf.go

@@ -59,6 +59,11 @@
 		}
 	}()

+	wafDiags := newHandle.Diagnostics()
+	if err := wafDiags.TopLevelError(); err != nil {


🚫 [golangci] _{reported by reviewdog 🐶}
wafDiags.TopLevelError undefined (type waf.Diagnostics has no field or method TopLevelError)) (typecheck)

github-actions · 2023-11-14T13:24:50Z

internal/appsec/waf.go

@@ -59,6 +59,11 @@
 		}
 	}()

+	wafDiags := newHandle.Diagnostics()
+	if err := wafDiags.TopLevelError(); err != nil {


🚫 [golangci] _{reported by reviewdog 🐶}
wafDiags.TopLevelError undefined (type waf.Diagnostics has no field or method TopLevelError) (typecheck)

github-actions · 2023-11-14T13:24:51Z

internal/appsec/waf.go

@@ -59,6 +59,11 @@
 		}
 	}()

+	wafDiags := newHandle.Diagnostics()
+	if err := wafDiags.TopLevelError(); err != nil {


🚫 [golangci] _{reported by reviewdog 🐶}
wafDiags.TopLevelError undefined (type waf.Diagnostics has no field or method TopLevelError)) (typecheck)

RomainMuller · 2023-11-14T13:59:08Z

/merge

dd-devflow · 2023-11-14T13:59:14Z

🚂 MergeQueue

Pull request added to the queue.

This build is going to start soon! (estimated merge in less than 10m)

you can cancel this operation by commenting your pull request with /merge -c!

dd-devflow · 2023-11-14T14:09:48Z

🚨 MergeQueue

not able to merge the branch in the target branch

Details

Error: PUT https://api.github.com/repos/DataDog/dd-trace-go/pulls/2338/merge: 405 5 of 5 required status checks are expected. []

FullStacktrace:
activity error (type: github.GithubService_MergePullRequest, scheduledEventID: 41, startedEventID: 42, identity: 1@github-worker-68bf997945-6hkzp@): PUT https://api.github.com/repos/DataDog/dd-trace-go/pulls/2338/merge: 405 5 of 5 required status checks are expected. [] (type: ErrorResponse, retryable: true)

If you need support, contact us on slack #ci-interfaces with those details!

Upgrades `github.com/DataDog/go-libddwaf` to the first commit that includes `libddwaf@1.5`, which is capable of ephemeral addresses, allowing workloads such as gRPC streaming and GraphQL to evaluate certain rules multiple times without having to reset the entire WAF context each time.

…erals

…g/go-libddwaf/v2 Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>

Co-authored-by: Julio Guerra <julio@datadog.com>

This reverts commit 3b3a2f3.

eliottness · 2023-11-14T14:23:53Z

/merge

dd-devflow · 2023-11-14T14:23:59Z

🚂 MergeQueue

Pull request added to the queue.

This build is going to start soon! (estimated merge in less than 10m)

you can cancel this operation by commenting your pull request with /merge -c!

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com> Co-authored-by: Eliott Bouhana <eliott.bouhana@datadoghq.com> Co-authored-by: Julio Guerra <julio@datadog.com>

dd-devflow · 2023-11-14T14:33:21Z

🚂 MergeQueue

PullRequest was merged manually

RomainMuller commented Nov 7, 2023

View reviewed changes

contrib/google.golang.org/grpc/appsec_test.go Outdated Show resolved Hide resolved

RomainMuller marked this pull request as ready for review November 7, 2023 17:35

RomainMuller requested review from a team as code owners November 7, 2023 17:35

Julio-Guerra requested changes Nov 8, 2023

View reviewed changes

github-actions bot reviewed Nov 9, 2023

View reviewed changes

internal/appsec/waf_unit_test.go Outdated Show resolved Hide resolved

internal/appsec/waf_unit_test.go Outdated Show resolved Hide resolved

Julio-Guerra reviewed Nov 13, 2023

View reviewed changes

internal/appsec/waf.go Show resolved Hide resolved

Julio-Guerra approved these changes Nov 14, 2023

View reviewed changes

RomainMuller enabled auto-merge (squash) November 14, 2023 13:23

RomainMuller disabled auto-merge November 14, 2023 13:23

github-actions bot reviewed Nov 14, 2023

View reviewed changes

RomainMuller enabled auto-merge (squash) November 14, 2023 13:28

RomainMuller disabled auto-merge November 14, 2023 13:58

dd-devflow bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: error and removed mergequeue-status: queued mergequeue-status: in_progress labels Nov 14, 2023

RomainMuller added 8 commits November 14, 2023 15:12

don't reset the waf context mid-flight in gRPC now that we have ephem…

8c884d0

…erals

Don't skip applying actions when there are some

200f953

limit environment tampering to smallest possible window

5c0a07c

format status code as a string so the WAF actually examines it

d79918c

remove env var hijack

340eb3b

fixup RC update test

81041f2

switch to merged in commit

cd1b209

RomainMuller and others added 10 commits November 14, 2023 15:12

use DataDog/go-libddwaf#44

afcb76e

get the waf diagnosics on listener creation

a1e1819

remove atomic counters that are no longer necessary

63423d7

make stuff pass

6416557

fix import paths: github.com/DataDog/go-libddwaf -> github.com/DataDo…

268a1e8

…g/go-libddwaf/v2 Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>

upgrade go-libddwaf: 1.7.0 -> 2.1.0

12bd694

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>

Update internal/appsec/waf.go

ef9f2a5

Co-authored-by: Julio Guerra <julio@datadog.com>

debug-log any top-level error observed

61384ae

update to github.com/DataDog/appsec-internal-go@v1.0.2

6dcc269

Revert "debug-log any top-level error observed"

2aca4bc

This reverts commit 3b3a2f3.

eliottness force-pushed the romain.marcadier/waf-upgrade-ephemeral/APPSEC-12062 branch from dbaa2fc to 2aca4bc Compare November 14, 2023 14:12

RomainMuller enabled auto-merge (squash) November 14, 2023 14:13

auto-merge was automatically disabled November 14, 2023 14:19
Pull Request is not mergeable

dd-devflow bot added mergequeue-status: queued mergequeue-status: in_progress and removed mergequeue-status: error mergequeue-status: queued labels Nov 14, 2023

RomainMuller merged commit edc0344 into main Nov 14, 2023
344 checks passed

RomainMuller deleted the romain.marcadier/waf-upgrade-ephemeral/APPSEC-12062 branch November 14, 2023 14:27

dd-devflow bot added mergequeue-status: done and removed mergequeue-status: in_progress labels Nov 14, 2023

darccio restored the romain.marcadier/waf-upgrade-ephemeral/APPSEC-12062 branch November 16, 2023 10:22

darccio deleted the romain.marcadier/waf-upgrade-ephemeral/APPSEC-12062 branch November 16, 2023 10:24

Julio-Guerra added the appsec label Nov 16, 2023

Julio-Guerra changed the title ~~feat: upgrade in-app WAF to ephemeral-capable version~~ contrib/google.golang.org/grpc: improve the memory efficiency of threats detection for grpc Nov 16, 2023

github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

contrib/google.golang.org/grpc: improve the memory efficiency of threats detection for grpc #2338

contrib/google.golang.org/grpc: improve the memory efficiency of threats detection for grpc #2338

RomainMuller commented Nov 7, 2023

pr-commenter bot commented Nov 7, 2023 •

edited

github-actions bot Nov 14, 2023

github-actions bot Nov 14, 2023

github-actions bot Nov 14, 2023

github-actions bot Nov 14, 2023

github-actions bot Nov 14, 2023

RomainMuller commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

eliottness commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

contrib/google.golang.org/grpc: improve the memory efficiency of threats detection for grpc #2338

contrib/google.golang.org/grpc: improve the memory efficiency of threats detection for grpc #2338

Conversation

RomainMuller commented Nov 7, 2023

What does this PR do?

Motivation

Reviewer's Checklist

pr-commenter bot commented Nov 7, 2023 • edited

Benchmarks

github-actions bot Nov 14, 2023

Choose a reason for hiding this comment

github-actions bot Nov 14, 2023

Choose a reason for hiding this comment

github-actions bot Nov 14, 2023

Choose a reason for hiding this comment

github-actions bot Nov 14, 2023

Choose a reason for hiding this comment

github-actions bot Nov 14, 2023

Choose a reason for hiding this comment

RomainMuller commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

eliottness commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

dd-devflow bot commented Nov 14, 2023

pr-commenter bot commented Nov 7, 2023 •

edited