Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

appsec: setup ossec package and OpenOperation #2781

Merged
merged 7 commits into from
Aug 16, 2024

Conversation

eliottness
Copy link
Contributor

@eliottness eliottness commented Jul 10, 2024

What does this PR do?

This PR setups protection against LFI attacks:

  • Setup ossec listener with the new address and register the listener where it is needed
  • Setup ossec emitter package with the new OpenOperation
  • Fix GLS nil-map assignment issue
  • Skip stackframes with the file name <generated>
  • Add Orchestrion workflow runs-on parameter

Motivation

Complete support for ASM Exploit Prevention.

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!
Signed-off-by: Eliott Bouhana eliott.bouhana@datadoghq.com

@pr-commenter
Copy link

pr-commenter bot commented Jul 10, 2024

Benchmarks

Benchmark execution time: 2024-08-16 13:03:48

Comparing candidate commit 465bb69 in PR branch eliott.bouhana/APPSEC-53772 with baseline commit 0462924 in branch main.

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

scenario:BenchmarkStartRequestSpan-24

  • 🟩 execution_time [-8.200ns; -6.980ns] or [-2.671%; -2.274%]

@eliottness eliottness marked this pull request as ready for review July 10, 2024 14:52
@eliottness eliottness requested a review from a team as a code owner July 10, 2024 14:52
Base automatically changed from eliott.bouhana/APPSEC-53654 to main July 10, 2024 15:34
@eliottness eliottness requested review from a team as code owners July 10, 2024 15:34
An error occurred while trying to automatically change base from eliott.bouhana/APPSEC-53654 to main July 10, 2024 15:34
@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Jul 10, 2024
@eliottness eliottness force-pushed the eliott.bouhana/APPSEC-53772 branch 3 times, most recently from 68fe39e to c3342d8 Compare July 10, 2024 15:40
@eliottness eliottness marked this pull request as draft July 12, 2024 08:07
@eliottness eliottness force-pushed the eliott.bouhana/APPSEC-53772 branch from c3342d8 to f0db6f9 Compare July 18, 2024 13:43
internal/appsec/emitter/ossec/lfi.go Show resolved Hide resolved
internal/appsec/listener/httpsec/http.go Outdated Show resolved Hide resolved
internal/appsec/listener/grpcsec/grpc.go Outdated Show resolved Hide resolved
internal/appsec/listener/ossec/lfi.go Outdated Show resolved Hide resolved
internal/appsec/dyngo/operation.go Outdated Show resolved Hide resolved
@eliottness eliottness force-pushed the eliott.bouhana/APPSEC-53772 branch from f0db6f9 to a6344bf Compare August 7, 2024 15:36
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
@eliottness eliottness force-pushed the eliott.bouhana/APPSEC-53772 branch from 104538f to 4e7459c Compare August 13, 2024 16:04
@eliottness eliottness marked this pull request as ready for review August 13, 2024 16:04
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
@eliottness eliottness force-pushed the eliott.bouhana/APPSEC-53772 branch from 7a9527d to 71f22a2 Compare August 13, 2024 16:30
@eliottness
Copy link
Contributor Author

Copy link
Contributor

@RomainMuller RomainMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Modulo the still open conversation :)

…er + refactor OpenOperationRes

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
@eliottness eliottness merged commit ea13a82 into main Aug 16, 2024
186 checks passed
@eliottness eliottness deleted the eliott.bouhana/APPSEC-53772 branch August 16, 2024 13:44
darccio pushed a commit that referenced this pull request Aug 19, 2024
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
github-merge-queue bot pushed a commit to DataDog/orchestrion that referenced this pull request Aug 29, 2024
### What does this PR do?

This PR instrument all file opening via the `os.Openfile` function for
RASP LFI and is the sister PR of
DataDog/dd-trace-go#2781

- [x] Write intrumentation file
- [x] Write integration tests for the `os` package
- [x] Add `tracer-internal: true` to the `dd:orchestrion-enabled`
instrumentation to properly enable the GLS storage in dd-trace-go
- [x] Add more tests for `dd:orchestrion-enabled`    

### Motivation

Support for LFI protection.

### Reviewer's Checklist
<!--
* Authors can use this list as a reference to ensure that there are no
problems
during the review but the signing off is to be done by the reviewer(s).
-->

- [ ] Changed code has unit tests for its functionality.

---------

Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants