Rare/Unusual Domain & Destination Flow Check

[DropB1t]

Me and my colleagues @LeoBubi & @F3rto have implemented a check Flow Check for rare domains and destinations, using ndpi_bitmap , with periodic learning.

Mentioned issues are
#6416, #6417

[DropB1t]

Just in case, I've already fixed the conflicts with base branch. Looking forward to move on with pr

[cardigliano]

• It was required to implement one flow check for the domains, and one for IPs. It is ok using a single Check, but it is required to use at least two different bitmaps to keep the status.

• Currently the status (bitmaps) is kept in LocalHost, the issue required this to be per ntopng instance. I suggest moving the bitmaps to NetworkInterface to make this global per interface.

• The learning mechanism is not optimal (besides the fact that code itself is a bit too complicated and can be simplified):
  The algorithm it is based on periodic learning periods, during those periods the check is in standby mode and it does not detect/trigger alerts. I suggest reworking this to have a single standby period during the initial learning, after this period all the new destinations should be just added to the status/bitmaps when running the check. The status should be still serialized on redis periodically.

Please also check my comments inline in the code.

Thank you.

[LeoBubi]

Thank you for the precious comments and suggestions, we're going to fix the code as soon as possible.

We only have some doubts about the learning mechanism you suggested. The periodic training logic was implemented because we think that if a destination is not seen in a while then it makes sense to consider it rare again. If only a single initial training is done, it is not possible to do such thing, causing the bitmaps to fill up after some time, effectively making the check useless. In addition, we talked to @lucaderi some time ago, and he suggested that we proceed by implementing periodic training.

Regarding the bitmaps in LocalHost, @lucaderi said that it was up to us to decide whether to do it for host or network interface. Anyway, we'll move the bitmaps to NetworkInterface as suggested.

[cardigliano]

As of the periodic training, what I proposed is the easiest solution: just warn once for every rare destination detected, and ignore it for the future. Alternatively you can periodically rebuild the bitmaps, but this should happen while running the check (building a "shadow" bitmap and swapping the bitmaps after the learning period), without putting the check in "standby" during the learning periods as this is like disabling the check for several hours per day.

[lucaderi]

Thank you for the precious comments and suggestions, we're going to fix the code as soon as possible.

We only have some doubts about the learning mechanism you suggested. The periodic training logic was implemented because we think that if a destination is not seen in a while then it makes sense to consider it rare again. If only a single initial training is done, it is not possible to do such thing, causing the bitmaps to fill up after some time, effectively making the check useless. In addition, we talked to @lucaderi some time ago, and he suggested that we proceed by implementing periodic training.

Regarding the bitmaps in LocalHost, @lucaderi said that it was up to us to decide whether to do it for host or network interface. Anyway, we'll move the bitmaps to NetworkInterface as suggested.

A rare destination is as such if it has not been detected so far in any of the hosts currently monitored. The learning period (or grace period) is a way to avoid triggering alerts until some basic knowledge is made. So it's done once at beginning and not periodically. Instead periodically you need to keep a backlog of visited hosts that allow you to trigger events for sites visited seldom (i.e. once a week). Does all this make sense to you?

[lucaderi]

@DropB1t Anything we can do to expedite this PR?

[DropB1t]

As suggested by the review:

• Status ( bitmaps ) has been decoupled: for domain & IPs
• Status has been moved into NetworkInterface, and now it's global for each interface
• Inline comments fixes has been made
• Learning mechanism has been streamlined

[cardigliano]

My comments after the last review:

• Two bitmaps (rare_dest_local and rare_dest_remote) are used now, why you need them? In my previous review I suggested to use two different bitmaps for domain names and IP addresses to avoid mixing them (and create collisions).
• Issues Rare/Unusual Domain Flow Check #6416 and Rare/Unusual Destination Flow Check #6417 ask to track domain names and destination IPs, it is not really clear what RareDestination::protocolDetected is actually aiming to check and in which cases the traffic is going to be evaluated, some comment would help, especially around this if statement https://github.com/ntop/ntopng/pull/7807/files#diff-571714b52e75685b8e6fdc55c8f88a1c7e5ffc6131f1c7d195561a4aa9d03d1cR59
• Similar to the previous item, RareDestination::getDestinationHash is not really clear, and there is some unhandled "else" condition. In general it is not clear which are all the cases this code is aiming to handle (referring to the values of src, dst, local, remote, mac, etc).

[DropB1t]

• As was suggested we had separated the circle of concern between local and remote destinations and created those two statuses ( bitmaps ). As we opted to handle also mac addresses ( in case if dhcp is turned on on the local network ) the given name ( rare_dest_local ) seems to be more suited, and more generic, for the case
• As for if statement on line 59 we try to perform the Check only if the client host is on the Local network and we have at our disposal either domain name or ip adress of the server host. If neither domain nor ip is available we cannot obtain the index key ( hash ) that is used to access the bitmaps
• In RareDestination::getDestinationHash we try to define what type of traffic we are looking at ( from Local to Local host or from Local to Remote host ) and differentiate the destination type ( destType ) to select the appropriate bitmap to perform on, later in the Check. As stated in the first item we tried to use the mac addresses as hash key in case the DHCP is on and the server host is neither Brodcast nor Multicast Host. If not the hash key will be one of IPs at our disposal. In case of remote destination we try to set the hash key with the hash of the domain name using Utils::hashString(f->getFlowServerInfo()) ( I searched the appropriate method, in the flow interface at our disposal, to retrieve the domain name of the remote destination and f->getFlowServerInfo() seemed to be the right one, correct me if I had done the wrong choice )

I hope I have explained myself well. Thank you for your attention and time

[lucaderi]

Closing for inactivity