Filter out Suricata rules when assembling zdeps on Windows#2858
Merged
Conversation
nwt
reviewed
Oct 16, 2023
| path.resolve("..", "..", "node_modules", "brimcap", "build", "dist"), | ||
| zdepsPath | ||
| zdepsPath, | ||
| { filter: filterBrimcapZdeps } |
Member
There was a problem hiding this comment.
Nits: I'd just use an arrow function. And no need for an if statement. And I'd just use one regular expression.
Non-nit: fs.statSync is much slower than the other tests so call it last.
Suggested change
| { filter: filterBrimcapZdeps } | |
| { | |
| // Drop Suricata rules on Windows to fix false positive malware flagging. | |
| // See https://github.com/brimdata/zui/issues/2857. | |
| filter: (src, _dest) => | |
| process.platform == "win32" && | |
| src.match(/(emerging\.rules\.tar\.gz|suricata\.rules)$/) && | |
| fs.statSync(src).isFile(), | |
| } |
Member
There was a problem hiding this comment.
@philrz: My original suggestion had some bad syntax and bad formatting so I updated it.
philrz
force-pushed
the
drop-suricata-rules-on-windows
branch
from
e579e01 to
8c9bbf0
Compare
nwt
approved these changes
Oct 16, 2023
philrz
added a commit
that referenced
this pull request
Oct 16, 2023
This was referenced Oct 16, 2023
Merged
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
2 participants
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.
When investigating the reports linked from #2857, one of the high-level patterns observed was reference to the Suricata rules that ship with Zui as part of Brimcap. Thinking these might be a root cause of the many evidence-of-malware flaggings found in the reports, one of our community members that works in the cybercrime team at Mandiant/Google confirmed our suspicion that this might be a root cause of some/all of the problems.
The shipped fallback rule sets have always been "nice to have" because they'd potentially allow some initial alerts even when installing Zui in an airgapped environment. However, these rules are arguably out-of-date by the time the user installs Zui since they're frozen at the time Zui is built & shipped. Meanwhile, almost all users are surely Internet connected and will hence get up-to-date rules the first time they launch Zui. So as much as we'd prefer it if all the vendors saw the rules as harmless, the approach in this PR takes the shortcut of just dropping the rules files from the Windows build.
With a Dev Build based on this branch, this VirusTotal report shows zero out of 66 vendors flagging it for evidence of malware, compared with the 16/69 shown with the GA Zui v1.3.0 build in #2857.
Likewise this Recorded Future Triage report shows a score of 4/10 (which is not shown in "red"), compared with the 10/10 shown with the GA Zui v1.3.0 build in #2857.
Tests I've run with the Dev Build:
suricata.rulesand70d9eddbf429eafe2b741e615a00a74a-emerging.rules.tar.gzfiles are the only files dropped from the installed Dev build when compared to a GA v1.3.0 installFixes #2857