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

Filter out Suricata rules when assembling zdeps on Windows #2858

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

philrz
Copy link
Contributor

@philrz philrz commented Oct 16, 2023

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 sandbox detections on the rulesets makes sense to me, ive seen that problem with other rulesets in the past where AV sigs are too loose and hit on detections for malware instead of the malware itself.

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.

image

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.

image

Tests I've run with the Dev Build:

  1. Confirmed on Windows that the suricata.rules and 70d9eddbf429eafe2b741e615a00a74a-emerging.rules.tar.gz files are the only files dropped from the installed Dev build when compared to a GA v1.3.0 install
  2. Confirmed on Windows when running the Dev Build that the updated-over-the-Internet Suricata rules end up being the same as the when running the GA v1.3.0 install
  3. Confirmed that the shipped rules files are still present on a macOS install of the Dev Build

Fixes #2857

@philrz philrz requested review from mattnibs, nwt and jameskerr October 16, 2023 18:06
@philrz philrz self-assigned this Oct 16, 2023
async function main() {
try {
fs.copySync(
path.resolve("..", "..", "node_modules", "brimcap", "build", "dist"),
zdepsPath
zdepsPath,
{ filter: filterBrimcapZdeps }
Copy link
Member

@nwt nwt Oct 16, 2023

Choose a reason for hiding this comment

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

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(),
}

Copy link
Member

Choose a reason for hiding this comment

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

@philrz: My original suggestion had some bad syntax and bad formatting so I updated it.

@philrz philrz force-pushed the drop-suricata-rules-on-windows branch from e579e01 to 8c9bbf0 Compare October 16, 2023 19:53
@philrz philrz merged commit 02b2495 into main Oct 16, 2023
6 checks passed
@philrz philrz deleted the drop-suricata-rules-on-windows branch October 16, 2023 20:23
@philrz philrz linked an issue Oct 26, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants