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

Analyzers that exit clean without reading input until EOF #331

Closed
philrz opened this issue Jan 23, 2024 · 3 comments · Fixed by #332
Closed

Analyzers that exit clean without reading input until EOF #331

philrz opened this issue Jan 23, 2024 · 3 comments · Fixed by #332
Assignees

Comments

@philrz
Copy link
Contributor

philrz commented Jan 23, 2024

@nwt's comment from brimdata/zui#2926 (comment):

What's happening here is that Suricata is exiting cleanly (i.e., with status 0) without reading its standard input to EOF. When Suricata exits, io.Copy in the goroutine created by analyzer.runProcesses returns because writes to the pipe connected to its standard input start failing. The goroutine then closes Zeek's standard input, resulting in this error.

That Zui issue improved how Brimcap catches and presents this kind of error, but @nwt feels there's still improvements that can be made on the Brimcap side. A current quote:

Right now brimcap assumes that analyzers that exit with status zero will read their standard input until EOF. I’m not sure that’s a good assumption.

@philrz
Copy link
Contributor Author

philrz commented Jan 24, 2024

Note to self: I think when this one is fixed, the user's pcap referenced in brimdata/zui#2926 (comment) should be able to load successfully in Zui. Suricata will still decline to analyze it (but exit with code 0) because of this open Suricata issue, but it sounds like an inaccurate Zeek failure will no longer be surfaced, which seems like an improvement to me.

@philrz
Copy link
Contributor Author

philrz commented Jan 24, 2024

In fact, the same goes for the pcap referenced in brimdata/zui#2926 (comment). I'm late in fully recognizing this, but that one's similarly mis-reported as a Zeek failure at the moment but due to a different open Suricata issue. Zeek still doesn't do anything interesting with this traffic (it generates a couple weird and notice events that express its inability to handle 802.11) but it seems to finish processing and exits cleanly.

mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 25, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
mattnibs added a commit that referenced this issue Jan 29, 2024
This commit changes the behavior for analyzer processes so that
processes that have successfully exited without reading all the data
will continue to consume data from the byte stream insteading of
returning an error and putting a stop to the copy goroutine.

Closes #331
@philrz
Copy link
Contributor Author

philrz commented Jan 29, 2024

Verified in Brimcap commit 070d2a0.

In the attached video I'm testing it out by using Zui commit 92dcd10 pointed at Brimcap commit 070d2a0 to have the benefit of this fix. Loading the VanSpy.pcapng test file from brimdata/zui#2926 (comment), this now loads without popping up an error. I also successfully loaded the pcap mentioned in brimdata/zui#2926 (comment) but since that user's data was sensitive it's not shown in the video.

Verify.mp4

Thanks @mattnibs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants