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

Address diagnostics #221

Merged
merged 4 commits into from
Oct 13, 2023
Merged

Address diagnostics #221

merged 4 commits into from
Oct 13, 2023

Conversation

Anilm3
Copy link
Collaborator

@Anilm3 Anilm3 commented Oct 11, 2023

This PR introduces information regarding addresses to the diagnostics for each of the different features. This includes both required and optional addresses, for example:

  "processors": {
    "loaded": [
      "processor-001"
    ],
    "failed": [],
    "errors": {},
    "addresses": {
      "required": [ ... ],
      "optional": [ ... ]
    }
  },

This allows the WAF user to distinguish which addresses they actually need to provide (required) from those which are opportunistically used (optionals) . It also allows distinguishing addresses which are used in potentially disabled features, for example, the processors section would provide insights into the addresses used for schema extraction:

  • Required refers to the condition addresses.
  • Optional refers to the mappings used to generate schemas, these are not "required" to run the processor but if available they will be used.

Unfortunately, it's not possible to distinguish which addresses correspond to the schema extraction processor, although if more processors are included in the future, this might be required.

Finally, this PR also renames ddwaf_required_addresses to ddwaf_known_addresses to better reflect the actual contents. This function now also returns optional addresses (processor mappings, object filter addresses) and required addresses (mainly condition addresses).

Related Jira APPSEC-11125

@Anilm3 Anilm3 requested a review from a team as a code owner October 11, 2023 20:33
@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2023

Codecov Report

Merging #221 (d967638) into master (0559fc0) will increase coverage by 0.00%.
The diff coverage is 84.70%.

@@           Coverage Diff           @@
##           master     #221   +/-   ##
=======================================
  Coverage   82.56%   82.57%           
=======================================
  Files         106      106           
  Lines        4004     4069   +65     
  Branches     1823     1859   +36     
=======================================
+ Hits         3306     3360   +54     
+ Misses        279      277    -2     
- Partials      419      432   +13     
Flag Coverage Δ
waf_test_none 82.46% <84.70%> (+0.01%) ⬆️
waf_test_sse2 82.54% <84.70%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/interface.cpp 64.70% <100.00%> (ø)
src/processor.hpp 100.00% <100.00%> (ø)
src/ruleset_builder.cpp 80.64% <100.00%> (-0.18%) ⬇️
src/ruleset_builder.hpp 100.00% <ø> (ø)
src/parser/parser_v2.cpp 87.45% <96.77%> (+0.58%) ⬆️
src/ruleset_info.cpp 97.22% <92.85%> (-2.78%) ⬇️
src/ruleset.hpp 66.66% <72.22%> (+2.56%) ⬆️
src/ruleset_info.hpp 73.97% <60.00%> (-3.62%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Anilm3 Anilm3 marked this pull request as draft October 11, 2023 20:52
@Anilm3 Anilm3 marked this pull request as ready for review October 12, 2023 16:08
@Anilm3 Anilm3 merged commit 433bb75 into master Oct 13, 2023
25 checks passed
@Anilm3 Anilm3 deleted the anilm3/address-diagnostics branch October 13, 2023 09:01
Copy link
Collaborator

@Taiki-San Taiki-San left a comment

Choose a reason for hiding this comment

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

lgtm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants