Fix False SYNFLOOD alerts

ntop · Oct 2, 2024 · e7981bb · e7981bb
1 parent 2cf7ba7
commit e7981bb
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 13 deletions.
diff --git a/include/AlertCounter.h b/include/AlertCounter.h
@@ -44,6 +44,8 @@ class AlertCounter {
  public:
   AlertCounter();
   void inc(time_t when, AlertableEntity *alertable);
+  void inc_no_time_window();
+  void dec();
   u_int16_t hits() const;
   void reset_hits();
 };

diff --git a/include/Flow.h b/include/Flow.h
@@ -1405,6 +1405,7 @@ inline float get_goodput_bytes_thpt() const { return (goodput_bytes_thpt); };
   inline MinorConnectionStates getCurrentConnectionState() { return(current_c_state); };
   bool checkS1ConnState();
   bool isTCPFlagSet(u_int8_t flags, int flag_to_check);
+  void updateSynFloodAlerts(bool connection_opened); 
   MinorConnectionStates calculateConnectionState(bool is_cumulative);
   MajorConnectionStates getMajorConnState();
   inline u_int32_t getPreNATSrcIp() { return ntohl(src_ip_addr_pre_nat); };

diff --git a/include/Host.h b/include/Host.h
@@ -540,6 +540,7 @@ class Host : public GenericHashEntry,
   bool addIfMatching(lua_State *vm, AddressTree *ptree, char *key);
   bool addIfMatching(lua_State *vm, u_int8_t *mac);
   void updateSynAlertsCounter(time_t when, bool syn_sent);
+  void updateSynFloodAlertsCounter(bool attacker, bool connection_opened);
   void updateFinAlertsCounter(time_t when, bool fin_sent);
   void updateRstAlertsCounter(time_t when, bool rst_sent);
   void updateICMPAlertsCounter(time_t when, bool icmp_sent);

diff --git a/include/NetworkStats.h b/include/NetworkStats.h
@@ -138,6 +138,7 @@ class NetworkStats : public InterfaceMemberAlertableEntity,
   virtual void updateStats(const struct timeval *tv);
 
   void updateSynAlertsCounter(time_t when, bool syn_sent);
+  void updateSynFloodAlertsCounter(bool connection_opened);
   void updateSynAckAlertsCounter(time_t when, bool synack_sent);
   void updateRoundTripTime(u_int32_t rtt_msecs);
   void incNumFlows(time_t t, bool as_client);

diff --git a/scripts/locales/en.lua b/scripts/locales/en.lua
@@ -1083,8 +1083,8 @@ local lang = {
     ["subject_quota_exceeded"] = "Host pool <a href=\"%{url}\">%{pool}</a> exceeded %{subject} quota [%{value} &gt; %{quota}]",
     ["suspicious_dga_domain_http"] = "Possible risky DGA Domain name detected [url: %{href}]",
     ["suspicious_dga_domain_other"] = "Possible risky DGA Domain name detected [domain: %{href}]",
-    ["syn_flood_attacker"] = "%{entity} is a SYN flooder [%{value} &gt; %{threshold} SYN/sec sent for ~3 sec]",
-    ["syn_flood_victim"] = "%{entity} is under SYN flood attack [%{value} &gt; %{threshold} SYN/sec received for ~3 sec]",
+    ["syn_flood_attacker"] = "%{entity} is a SYN flooder [%{value} &gt; %{threshold} half-opened TCP Connections]",
+    ["syn_flood_victim"] = "%{entity} is under SYN flood attack [%{value} &gt; %{threshold} half-opened TCP Connections]",
     ["syn_scan_attacker"] = "%{entity} is a SYN scan attacker [%{value} &gt; %{threshold} SYN sent]",
     ["syn_scan_victim"] = "%{entity} is under a SYN scan [%{value} &gt; %{threshold} SYN received]",
     ["system_error"] = "System error detected in ntopng, please report it. Error message: %{system_error_msg}",

diff --git a/src/AlertCounter.cpp b/src/AlertCounter.cpp
@@ -67,7 +67,15 @@ void AlertCounter::inc(time_t when, AlertableEntity *alertable) {
           );
   #endif
 }
+/* *************************************** */
+
+void AlertCounter::inc_no_time_window(){
+  current_hits++;
+}
 
+void AlertCounter::dec(){
+  current_hits--;
+}
 /* *************************************** */
 
 u_int16_t AlertCounter::hits() const {
@@ -81,4 +89,4 @@ u_int16_t AlertCounter::hits() const {
 
 /* *************************************** */
 
-void AlertCounter::reset_hits() { hits_reset_req = true; }
+void AlertCounter::reset_hits() { hits_reset_req = true; }
diff --git a/src/Flow.cpp b/src/Flow.cpp
@@ -445,6 +445,13 @@ Flow::~Flow() {
                       constructor (viewed interfaces) */
     delete srv_ip_addr;
 
+  /* Clean up SYN Flood Counts if the we have an open conection and the flow is being purged*/
+
+  if( getCurrentConnectionState() == S0 ){
+    updateSynFloodAlerts(false);
+  }
+
+
   /*
     Finish deleting other flow data structures
   */
@@ -8733,6 +8740,27 @@ bool Flow::checkS1ConnState() {
 				   !(isTCPFlagSet(src2dst_tcp_flags,TH_RST)) && !(isTCPFlagSet(dst2src_tcp_flags,TH_RST))     /* NO RST */
 				   ));
 }
+/* **************************************************** */
+
+void Flow::updateSynFloodAlerts(bool connection_opened){
+    if((current_c_state != S0 && !connection_opened) || 
+    (current_c_state !=MINOR_NO_STATE && connection_opened))
+      return;
+
+
+    NetworkStats *cli_network_stats = NULL, *srv_network_stats = NULL;
+    if (srv_host) 
+      srv_network_stats =
+        srv_host->getNetworkStats(srv_host->get_local_network_id());
+
+    if (cli_host)
+      cli_host->updateSynFloodAlertsCounter(true,connection_opened);
+    if (srv_host)
+      srv_host->updateSynFloodAlertsCounter(false,connection_opened);
+
+    if (srv_network_stats)
+      srv_network_stats->updateSynFloodAlertsCounter(connection_opened);
+}
 
 /* **************************************************** */
 
@@ -8745,19 +8773,39 @@ MinorConnectionStates Flow::calculateConnectionState(bool is_cumulative) {
       !(isTCPFlagSet(dst2src_tcp_flags,TH_SYN))) {
     if (!(isTCPFlagSet(dst2src_tcp_flags,TH_RST))) {
       if ((isTCPFlagSet(src2dst_tcp_flags,TH_RST))) {
+        updateSynFloodAlerts(false);
         return(setCurrentConnectionState(RSTOS0));
       } else {
         if ((isTCPFlagSet(src2dst_tcp_flags,TH_FIN))) {
+          updateSynFloodAlerts(false);
           return(setCurrentConnectionState(SH));
         } else {
-          return(setCurrentConnectionState(S0));
+            /* We are assuming that the TCP connection is established on
+             * server side when it sees the clients SYN
+             * this helps us detect SYN flood victims even if all the SYN
+             * packets are rejected or dropped*/
+            updateSynFloodAlerts(true);
+            return(setCurrentConnectionState(S0));
         }
       }
     } else {
+      updateSynFloodAlerts(false);
       return(setCurrentConnectionState(REJ));
     }
   }
 
+  /* Account for connection being closed after SYNACK*/
+  if ((isTCPFlagSet(src2dst_tcp_flags,TH_SYN)) &&
+      (isTCPFlagSet(dst2src_tcp_flags,TH_SYN)) &&
+      (isTCPFlagSet(dst2src_tcp_flags,TH_ACK)) &&
+      !(isTCPFlagSet(src2dst_tcp_flags,TH_ACK))&&
+      ((isTCPFlagSet(dst2src_tcp_flags,TH_RST))|| 
+      (isTCPFlagSet(src2dst_tcp_flags,TH_RST)) ||
+      (isTCPFlagSet(src2dst_tcp_flags,TH_FIN)) ||
+      (isTCPFlagSet(dst2src_tcp_flags,TH_FIN)))) {
+      updateSynFloodAlerts(false);
+  }
+
   /* Check RSTRH */
   if ((isTCPFlagSet(dst2src_tcp_flags,TH_SYN)) &&
       (isTCPFlagSet(dst2src_tcp_flags,TH_ACK)) &&
@@ -8810,8 +8858,10 @@ MinorConnectionStates Flow::calculateConnectionState(bool is_cumulative) {
     return(setCurrentConnectionState(RSTR));
 
   /* Check S1 */
-  if (checkS1ConnState())
+  if (checkS1ConnState()){
+    updateSynFloodAlerts(false);
     return(setCurrentConnectionState(S1));
+  }
 
   if (getCurrentConnectionState() != MINOR_NO_STATE)
     return(getCurrentConnectionState());

diff --git a/src/Host.cpp b/src/Host.cpp
@@ -177,10 +177,6 @@ u_int16_t Host::decScoreValue(u_int16_t score_decr,
 /* *************************************** */
 
 void Host::updateSynAlertsCounter(time_t when, bool syn_sent) {
-  AlertCounter *counter = syn_sent ? syn_flood.attacker_counter : syn_flood.victim_counter;
-
-  counter->inc(when, this);
-
   if (syn_sent)
     syn_scan.syn_sent_last_min++;
   else
@@ -189,6 +185,16 @@ void Host::updateSynAlertsCounter(time_t when, bool syn_sent) {
 
 /* *************************************** */
 
+void Host::updateSynFloodAlertsCounter(bool attacker, bool connection_opened) {
+  AlertCounter *counter = attacker ? syn_flood.attacker_counter : syn_flood.victim_counter;
+  if(connection_opened)
+    counter->inc_no_time_window();
+  else
+    counter->dec();
+}
+
+/* *************************************** */
+
 void Host::updateFinAlertsCounter(time_t when, bool fin_sent) {
   fin_sent ? fin_scan.fin_sent_last_min++ : fin_scan.fin_recvd_last_min++;
 }

diff --git a/src/NetworkStats.cpp b/src/NetworkStats.cpp
@@ -196,7 +196,7 @@ void NetworkStats::updateRoundTripTime(u_int32_t rtt_msecs) {
 void NetworkStats::housekeepAlerts(ScriptPeriodicity p) {
   switch (p) {
     case minute_script:
-      flow_flood_victim_alert.reset_hits(), syn_flood_victim_alert.reset_hits();
+      flow_flood_victim_alert.reset_hits(); /*,syn_flood_victim_alert.reset_hits()*/
       syn_recvd_last_min = synack_sent_last_min = 0;
       break;
     default:
@@ -208,10 +208,19 @@ void NetworkStats::housekeepAlerts(ScriptPeriodicity p) {
 
 void NetworkStats::updateSynAlertsCounter(time_t when, bool syn_sent) {
   if (!syn_sent) {
-    syn_flood_victim_alert.inc(when, this);
     syn_recvd_last_min++;
   }
 }
+/* *************************************** */
+
+void NetworkStats::updateSynFloodAlertsCounter( bool connection_opened) {
+  if (connection_opened) {
+    syn_flood_victim_alert.inc_no_time_window();
+  }
+  else{
+    syn_flood_victim_alert.dec();
+  }
+}
 
 /* *************************************** */
 

diff --git a/src/host_checks/SYNFlood.cpp b/src/host_checks/SYNFlood.cpp
@@ -35,7 +35,7 @@ void SYNFlood::periodicUpdate(Host *h, HostAlert *engaged_alert) {
                          CLIENT_NO_RISK_PERCENTAGE);
 
   /* Reset counters once done */
-  h->reset_syn_flood_hits();
+  //h->reset_syn_flood_hits();
 }
 
 /* ***************************************************** */
diff --git a/tests/e2e b/tests/e2e