fix(sonicwall): recover addresses, firewall actions and event outcomes (+ committed contract test) - #2756
Merged
Conversation
The address step splits src and dst only as ip:port:interface. The grok step writes nothing unless every pattern matches and the filter then deletes the value, so ip:port (VPN negotiation events) and ip::interface (user logins) lose their address. When fw_action is the last key, the rescue grok cannot bound it and the fallback rename reads log.fwaction, while go-sdk v1.1.35 and later keep the underscore, so those events get no action and no outcome. Events whose firewall action is NA get no outcome at all, and the CEF header step captures its fields with a lone lazy pattern, which the grok step rejects. Each address shape now has its own step, and the original value is deleted only after an address was read. A fallback reads fw_action under both spellings. Events without a firewall decision get an outcome from the meaning of their message: allowed logins and completed IKEv2 negotiations give success, as does Connection Closed when bytes came back; bad credentials, RADIUS, LDAP, XAUTH and SSO failures and failed IKE negotiations give failure; policy and zone refusals give denied. The CEF header fields use a non-empty pattern. The administrator authentication-failure rule counted "Administrator login allowed" (29) as a failure and, through its message branch, SSO probe failures and policy denials. It now counts logins denied due to bad credentials with a failure outcome, its history counts failures from the same address, and alerts group by address, so a password spray is one alert instead of one per user name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Grouping by address only nests repeats: every attempt that passes the history check still creates a child alert. On the busiest day in the last week one deployment had 41,227 bad-credential denials from 41 addresses, which would create 40,756 alerts. Deduplicating by address creates 15 and keeps one alert per source for seven days. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#2743 shipped no in-repo test (all evidence was out-of-band playground). Adds sonicwall_contract_test.go: an engine-faithful grok model (mirrors the merged fortigate contract test) that replays 6 admin-auth logins through the ordered filter and asserts origin.ip/origin.port/log.eventCode/actionResult plus that the shipped admin_auth_failures where: clause fires on the bad-credential cases (32/33/200) and not on allowed (29) / policy-denied (986). Verified discriminating: PASSES on this filter, FAILS on the base v11 filter (origin.ip and actionResult absent). Built at #2743's head.
❌ Go dependencies check failedThere are outdated Go dependencies, or modules that could not be inspected. Script output |
✅ AI review — Approved with warningsOnly minor (medium/low) issues were found. They won't block the merge, but consider addressing them. ✅
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carries #2743 SonicWall filter+rule fix (CEF header non-empty pattern; per-shape src/dst grok with inCIDR guards; fw_action fallback; eventCode outcome mapping) PLUS a committed raw-replay contract test that #2743 was missing.
sonicwall_contract_test.go models the engine grok/kv/rename/trim/cast/add/delete steps (mirrors the merged fortigate contract test), replays 6 admin-auth logins, and asserts origin.ip/origin.port/log.eventCode/actionResult plus that the shipped admin_auth_failures where clause fires on bad-credential events (32/33/200) and not on allowed (29) / policy-denied (986).
Verified discriminating: PASSES on this filter, FAILS on the base v11 filter (origin.ip and actionResult absent). Merging this subsumes #2743; #2743 can be closed.