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

Update to libddwaf 1.5.1 #2306

Merged
merged 27 commits into from
Oct 17, 2022
Merged

Update to libddwaf 1.5.1 #2306

merged 27 commits into from
Oct 17, 2022

Conversation

lloeki
Copy link
Contributor

@lloeki lloeki commented Oct 5, 2022

What does this PR do?

Update libddwaf to 1.5.1

Motivation

Support for new features, some fixes, and performance improvements.

How to test the change?

There should be specs + system tests covering these.

@github-actions github-actions bot added appsec Application Security monitoring product integrations Involves tracing integrations labels Oct 5, 2022
@lloeki lloeki mentioned this pull request Oct 11, 2022
@lloeki lloeki changed the base branch from master to 1.5-stable October 11, 2022 18:55
Also update vendored rulesets to 1.4.1

Notable libddwaf API changes include:
- ability to set sideband rule data
- ability to toggle rules
- proper separation of return code vs decided action
- ability to hint at multiple actions
- non-reliance on garbage collector (finalization must be explicit)
While the presence of a ruby-platform gem for these should be picked up
by bundler, sometimes it is not. Also, this makes it a bit more
future-proof, so that bundler doesn't attempt to pick a version that has
no ruby platform gem, and then proceed to fail.
While the presence of a ruby-platform gem for these should be picked up
by bundler, sometimes it is not. Also, this makes it a bit more
future-proof, so that bundler doesn't attempt to pick a version that has
no ruby platform gem, and then proceed to fail.
Some AppSec spec examples have to be tested against rack-contrib
Previoulsy setting the tag would work but the change would be silently
dropped, resulting in the change being absent from the final trace.
Prevents accumulation of instrumentation middlewares if multiple
configure blocks are being called, like over app hot-reloading (e.g
Rails development mode, which rereads initializers) or across a sequence
of examples within a spec suite.

Since watchers are essentially static, they need only to be set up once
per process, ever.
Rack and Rails are lazily populating upon access. Depending on the
access pattern this may mean that body data would not be available to
AppSec for analysis. This is worked around by a call to the
side-effectful accessors.
Consequently, context will not be set in Rack env, trickling down to
either disablement or enablement of other instrumented calls,
consistently for the whole request.
This would apply to handle as well, yet currently handle is a value that
exists only once per application, so is never to be freed since there is
no place for its finalize to be called.
This covers the following AppSec integrations:
- Rack
- Rails
- Sinatra
@lloeki lloeki marked this pull request as ready for review October 11, 2022 19:44
@lloeki lloeki requested a review from a team October 11, 2022 19:44
JSONBodyParser replaces PostBodyContentTypeParser
With POST requests, a CSRF token is theoretically needed, but we have
none. Skip the filter, using the appropriate method depending on Rails
versions. Also the mock app may not have the filter defined.
Prior to 0.7 an argument is lacking to generate multipart requests
without uploading a file.
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

I've left a few comments.

Overall seems OK, but I would strongly suggest avoiding this level of extreme complex PRs next time -- this was extremely hard to follow and review, and took a very long time :(

@lloeki lloeki merged commit 6be3263 into 1.5-stable Oct 17, 2022
@lloeki lloeki deleted the update-libddwaf branch October 17, 2022 14:37
@lloeki lloeki added this to the 1.5.1 milestone Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appsec Application Security monitoring product integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants