Project

General

Profile

Bug #4489

Fix filtering mechanism in reporter

Added by Jan Mach about 1 year ago. Updated about 2 months ago.

Status:
New
Priority:
High
Assignee:
Category:
Development - Core
Target version:
Start date:
01/03/2019
Due date:
% Done:

0%

Estimated time:
To be discussed:

Description

Filtering mechanism in reporter has a flaw, that can cause that events that should be filtered out still make it to the final report. This is most probably due to the fact, that single IDEA event can contain multiple sources and it passes filtering in case there is any source, that should not be filtered out. This of course needs to be corrected, perhaps by adding secondary filter before rendering the appropriate report line.

This bug should be resolved promptly, because it was reported a while ago.

Associated revisions

Revision b50a3fb0 (diff)
Added by Jan Žerdík about 2 months ago

Changes in filtering events.

(Redmine issue: #4489)

Revision a29c4ae3 (diff)
Added by Jan Žerdík about 2 months ago

Modification of tests.

(Redmine issue: #4489)

Revision 41e38632
Added by Jan Mach about 2 months ago

Merged work from Jan Žerdík.

(Redmine issue: #4489)

History

#1 Updated by Pavel Kácha about 1 year ago

Note: On resolution, notify recipients of Message-Id: <>.

#2 Updated by Jan Mach 11 months ago

  • Target version changed from 2.4 to 2.5

#3 Updated by Jan Mach 7 months ago

  • Target version changed from 2.5 to 2.6

#4 Updated by Pavel Kácha 5 months ago

  • Assignee changed from Jan Mach to Jan Žerdík

After discussion it’s obvious secondary filter will not suffice. In fact, only part of the event (containing IP address) is matched, and we should not report it, but we should report the rest. Possible solutions:

1. Learn PySpect to actually search (along with reporting found positions in events), instead of only matching. However, JŽ correctly noted, that more complex conditions would need to return inconveniently complex data pointers or results, which would be also hard to interpret.
2. Cleanup rules, convert advanced to simple where feasible, solve for simple rules, not for advanced.
3. Consider widening simple rule possibilities, so we can convert more from advanced.
4. Match event, than match each unfiltered event for all sources separately (with respective Source.?.IP? replaced with one item array), so we know exactly for which source it matched and omit this source from report.

Seems like 4 is winner in terms of ROI. Maybe it will add some overhead, however if we don’t allow Blabla[x] rules (which don’t make much sense anyway), we will have 100% covered.

#5 Updated by Jan Mach 5 months ago

  • To be discussed changed from No to Yes

#6 Updated by Pavel Kácha 5 months ago

  • To be discussed changed from Yes to No

#7 Updated by Pavel Kácha 5 months ago

Mek, could you please look up vhor’s example?

#8 Updated by Jan Mach 3 months ago

  • Target version changed from 2.6 to 2.7

#9 Updated by Jan Žerdík 3 months ago

  • To be discussed changed from No to Yes

After more testing... The filtering does not let through sources that do match filter. Example that Mek send me was badly written when it checked equality with subnet instead of “IN” test.
On the contrary, filter does not allow event through when at least one source match because of implementation of “OP_IN” operation in pynspect BaseFilteringTreeTraverser. For example, if Source.IP4 return [192.168.0.1, 192.168.1.1] and filter test is “Source.IP4 in [192.168.0.0/24]” than event match and is filtered out.
Should we flip discussed solution and recheck sources if filter match (even if that is not really subject of this ticket)?

#10 Updated by Pavel Kácha 3 months ago

Yup, we need to fix this also. From video - fixes will go to Mentat-alt for at least two weeks of testing.

#11 Updated by Pavel Kácha 3 months ago

  • To be discussed changed from Yes to No

#12 Updated by Pavel Kácha about 2 months ago

  • To be discussed changed from No to Yes

#13 Updated by Pavel Kácha about 2 months ago

  • To be discussed changed from Yes to No

#14 Updated by Jan Žerdík about 2 months ago

  • To be discussed changed from No to Yes

#15 Updated by Pavel Kácha about 2 months ago

So needs some testing on alt (artifical events).

#16 Updated by Pavel Kácha about 2 months ago

  • To be discussed deleted (Yes)

Also available in: Atom PDF