fix: community rules follow-up — JA4 rules, tests, shadow logging#15
Merged
Conversation
…scription fix Addresses findings from post-merge eng review of PR #13: - Add new_ja4_burst and dga_cert_diversity rules to c2_malware.toml. These two rules use JA4-specific fields (ja4, ja4x) that a NetFlow sensor cannot see — the actual differentiator vs. volumetric-only detection. Both include a note that they only fire on completed TLS handshakes. - Fix ssh_connection_burst description: was "opened more than 20 SSH connections" but the rule uses count_distinct(dst_ip), which measures distinct destination hosts, not total connection count. Corrected to "connected to more than 20 distinct hosts via SSH". - Add TestMergeRules_BothNil: guards against panic if both community and custom rule sets are nil (edge case, not reachable in normal operation). - Add TestCommunityRules_FireThroughEvaluator (integration test): end-to-end verification that LoadCommunityRules → NewCustomRules → Evaluate fires an alert. Specifically triggers port_scan_detector with 21 distinct dst_ports. Guards against silent regression if the wiring in initPipeline breaks. - Add logShadowedRules helper in main.go: logs a warning when a user custom rule shadows a community rule of the same name. Called at startup and on SIGHUP. Previously silent; helps operators debug unexpected detection gaps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Post-merge follow-up for PR #13 (community detection rule library), addressing findings from eng review:
c2_malware.toml:new_ja4_burst(>20 distinct JA4 fingerprints from one src in 5m, sliding) anddga_cert_diversity(>40 distinct JA4x server cert fingerprints from one src in 5m, sliding). These are the only community rules that use JA4-specific fields — a NetFlow sensor cannot replicate them.ssh_connection_burst: rule usescount_distinct(dst_ip), measuring distinct destination hosts, not total connection count. Description corrected.logShadowedRuleshelper logs a warning when a user custom rule shadows a community rule by name. Called at startup and on SIGHUP. Previously silent.TestMergeRules_BothNil(nil/nil edge case) andTestCommunityRules_FireThroughEvaluator(end-to-end integration test: LoadCommunityRules → NewCustomRules → Evaluate → alert fires forport_scan_detector).Test Plan
go test ./internal/config/...— LoadCommunityRules loads 11 rules (was 9), NoDuplicateNames, MergeRules_* including BothNilgo test ./internal/anomaly/...— integration test fires port_scan_detector alert after 21 distinct dst_ports🤖 Generated with Claude Code