-
Notifications
You must be signed in to change notification settings - Fork 1.5k
DDS: Watchguard-Firebox: Update Pipeline #21881
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
base: master
Are you sure you want to change the base?
DDS: Watchguard-Firebox: Update Pipeline #21881
Conversation
8a17509 to
1dac143
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| %{regex(".*(?= attack)"):attack_type} attack (against | ||
| %{ip:network.destination.ip} )?from %{ip:network.client.ip} | ||
| detected.( %{integer:drop_packet_count} %{regex(".*(?= | ||
| packets)"):drop_packet_type} packets dropped since last alarm.)? | ||
| parse_rule_30000161 %{regex(".*(?= from)"):attack_type} from client %{ip:network.client.ip} detected. | ||
| packets)"):drop_packet_type} packets dropped since last alarm.)?( | ||
| (%{notSpace}))? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add whitespace before optional event code in grok rule
The newly added optional capture after the drop‑packet clause in matchRules does not consume the space that precedes event codes. The pattern is currently ...last alarm.)?((%{notSpace}))? so Grok expects a non‑space character immediately after the period. Messages of the form "… last alarm. (udp_flood_dos)" (as shown in the updated samples) therefore fail to match because the trailing space remains unmatched, and the whole rule fails. As a result, any attack alert that includes both drop counts and an event identifier will no longer be parsed. Insert an explicit space or \s* before the optional (%{notSpace}) so the rule accepts messages with a space before the code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have included the space in the actual parsing rule as ...last alarm.)?( (%{notSpace}))?, but when the pipeline is exported, the space does not appear in the .yaml file. However, the pipeline still handles the space correctly, as the test samples are being parsed properly.
What does this PR do?
Motivation
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged