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

detect/requires: reset sigerror flags for each rule #10237

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

jasonish
Copy link
Member

Fix issue where a bad rule after a skipped rule was recorded as skipped.

Also add a note about which version of Suricata contain this keyword.

Ticket: https://redmine.openinfosecfoundation.org/issues/6710

SV_BRANCH=OISF/suricata-verify#1610

"sigerror_ok" and "sigerror_requires" were not being reset after each
rule which could lead to a rule load error being incorrectly tracked
as skipped rather than failed.

Also initialize "skippedsigs" to 0 along with "goodsigs" and
"badsigs", while not directly related to this issue, could also throw
off some stats.

Ticket: OISF#6710
@jasonish
Copy link
Member Author

CI error does not seem related: ===> http-not09: Sub test #1: FAIL : expected 1 matches; got 0 for filter {'count': 1, 'match': {'event_type': 'http', 'http.http_user_agent': 'myscript'}}

@suricata-qa
Copy link

Information:

ERROR: QA failed on SURI_TLPW1_files_sha256.

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.error.http.parser 1108 724 65.34%

Pipeline 17732

@victorjulien victorjulien added this to the 8.0 milestone Jan 25, 2024
de_ctx->sigerror_silent = false;
de_ctx->sigerror_requires = false;
Copy link
Member

Choose a reason for hiding this comment

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

This is a better solution than what I had proposed. Looks great!

This was referenced Jan 25, 2024
@victorjulien victorjulien merged commit 8bf8131 into OISF:master Jan 25, 2024
53 of 89 checks passed
@victorjulien
Copy link
Member

Merged in #10244, thanks!

@catenacyber catenacyber mentioned this pull request Jan 25, 2024
@jasonish jasonish deleted the requires-skipped-failed/v1 branch August 15, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants