brimdata / zui

Zui is a powerful desktop application for exploring and working with data. The official front-end to the Zed lake.
https://www.brimdata.io/download/
Other
1.8k stars 132 forks source link

Clicking a Suricata alert no longer activates Packets button (regression at 3e0bfbc) #2851

Open philrz opened 1 year ago

philrz commented 1 year ago

This issue can be reproduced with current tip of main (commit 7c8e523) but a binary search has confirmed that the problem started happening at commit 3e0bfbc, which is associated with the changes in #2455.

The first video shows when it was working ok at the prior commit 8752826. The attached pcap ifconfig.pcapng.gz (after uncompressing) is dragged into the app. Clicking on the alert event, we can see the Packets button is activated. When clicked, the underlying connection that generated the alert is opened in Wireshark.

https://github.com/brimdata/zui/assets/5934157/71cc236e-7ed6-4d02-87e3-9a62752579b0

This next video shows the same repro steps at the following commit, 3e0bfbc. As we can see, the Packets button is no longer activated when we click on the alert event. We are, however, able to find the correlated conn record in the Detail view and we can ultimately click on that and open Wireshark.

https://github.com/brimdata/zui/assets/5934157/24f9d783-8ce8-40aa-8658-a7269efe9070

Fast-forwarding to current Zui 7c8e523 where the problem can be similarly reproduced, I'm pretty sure the problem lies here:

https://github.com/brimdata/zui/blob/7c8e5238851fabc93d2de192da2e32c7e2ed80ea/apps/zui/src/plugins/brimcap/packets/menu-item.ts#L21

I added some debug console logging and confirmed this is being called every time I click on a record as a prerequisite for activating the Packets button. Suricata alerts have community_id, but not uid, so it makes sense that I saw findUid() returning null when I clicked on an alert. I'm not intimately familiar with the specifics of the code before the changes in #2455, but I seem to recall there was a logic chain upon clicking a Suricata event to check for a related conn record that shared a community_id, and maybe we lost that? Just a first guess.

philrz commented 1 year ago

Note to self: We should write a test for this.