Fixed syslog false positives. (#1577)

* syslog: removed unnecessary/unreliable printable string check
 * added `ndpi_isalnum()`
 * splitted `ndpi_is_printable_string()` into `ndpi_is_printable_buffer()` and `ndpi_normalize_printable_string()`

Signed-off-by: lns <matzeton@googlemail.com>

3 files changed, 48 insertions, 22 deletions

diff --git a/src/lib/ndpi_utils.c b/src/lib/ndpi_utils.c
index 4d7aedca3..f2cb9a4d5 100644
--- a/src/lib/ndpi_utils.c
+++ b/src/lib/ndpi_utils.c
@@ -755,8 +755,8 @@ static int _ndpi_is_valid_char(char c) {
   if(ispunct(c) && (!ndpi_is_other_char(c)))
     return(0);
   else
-    return(isdigit(c)
-	   || isalpha(c)
+    return(ndpi_isdigit(c)
+	   || ndpi_isalpha(c)
 	   || ndpi_is_other_char(c));
 }
 static char ndpi_is_valid_char_tbl[256],ndpi_is_valid_char_tbl_init=0;
@@ -2274,7 +2274,22 @@ int ndpi_isset_risk(struct ndpi_detection_module_struct *ndpi_str,
 
 /* ******************************************************************** */
 
-int ndpi_is_printable_string(char * const str, size_t len) {
+int ndpi_is_printable_buffer(uint8_t const * const buf, size_t len) {
+  int retval = 1;
+  size_t i;
+
+  for(i = 0; i < len; ++i) {
+    if(ndpi_isprint(buf[i]) == 0) {
+      retval = 0;
+    }
+  }
+
+  return retval;
+}
+
+/* ******************************************************************** */
+
+int ndpi_normalize_printable_string(char * const str, size_t len) {
   int retval = 1;
   size_t i;
 
diff --git a/src/lib/protocols/syslog.c b/src/lib/protocols/syslog.c
index 9722c92a0..406bf5e8f 100644
--- a/src/lib/protocols/syslog.c
+++ b/src/lib/protocols/syslog.c
@@ -38,13 +38,11 @@ void ndpi_search_syslog(struct ndpi_detection_module_struct
 			*ndpi_struct, struct ndpi_flow_struct *flow)
 {
   struct ndpi_packet_struct *packet = &ndpi_struct->packet;
-  u_int8_t i;
+  u_int16_t i;
 
   NDPI_LOG_DBG(ndpi_struct, "search syslog\n");
 
   if (packet->payload_packet_len > 20 && packet->payload[0] == '<') {
-    int j;
-
     NDPI_LOG_DBG2(ndpi_struct, "checked len>20 and <1024 and first symbol=<\n");
 
     for (i = 1; i <= 3; i++) {
@@ -70,18 +68,31 @@ void ndpi_search_syslog(struct ndpi_detection_module_struct
       NDPI_LOG_DBG2(ndpi_struct, "no blank following the >: do nothing\n");
     }
 
-    /* Even if there are 2 RFCs (3164, 5424), syslog format after "<NUMBER>" is
-       not standard. The only common pattern seems to be that the entire
-       payload is made by printable characters */
-    /* TODO: check only the first N bytes to avoid touching the entire payload? */
-    for (j = 0; j < packet->payload_packet_len - i; j++) {
-      if (!(ndpi_isprint(packet->payload[i + j]) ||
-            ndpi_isspace(packet->payload[i + j]))) {
-        NDPI_LOG_DBG2(ndpi_struct, "no printable char 0x%x [i/j %d/%d]\n",
-                      packet->payload[i + j], i, j);
-        NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
-        return;
-      }
+    while (i < packet->payload_packet_len)
+    {
+        if (ndpi_isalnum(packet->payload[i]) == 0)
+        {
+            if (packet->payload[i] == ' ' || packet->payload[i] == ':' ||
+                packet->payload[i] == '=')
+            {
+                break;
+            }
+            NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
+            return;
+        }
+
+        i++;
+    }
+
+    if (packet->payload[i] == ':')
+    {
+        i++;
+        if (i >= packet->payload_packet_len ||
+            packet->payload[i] != ' ')
+        {
+            NDPI_EXCLUDE_PROTO(ndpi_struct, flow);
+            return;
+        }
     }
 
     NDPI_LOG_INFO(ndpi_struct, "found syslog\n");
diff --git a/src/lib/protocols/tls.c b/src/lib/protocols/tls.c
index 6d9bc12ad..8a7359ad1 100644
--- a/src/lib/protocols/tls.c
+++ b/src/lib/protocols/tls.c
@@ -260,7 +260,7 @@ static int extractRDNSequence(struct ndpi_packet_struct *packet,
   buffer[len] = '\0';
 
   // check string is printable
-  is_printable = ndpi_is_printable_string(buffer, len);
+  is_printable = ndpi_normalize_printable_string(buffer, len);
 
   if(is_printable) {
     int rc = ndpi_snprintf(&rdnSeqBuf[*rdnSeqBuf_offset],
@@ -394,7 +394,7 @@ static void processCertificateElements(struct ndpi_detection_module_struct *ndpi
 
 	if(rdn_len && (flow->protos.tls_quic.issuerDN == NULL)) {
 	  flow->protos.tls_quic.issuerDN = ndpi_strdup(rdnSeqBuf);
-	  if(ndpi_is_printable_string(rdnSeqBuf, rdn_len) == 0) {
+	  if(ndpi_normalize_printable_string(rdnSeqBuf, rdn_len) == 0) {
 	    char str[64];
 
 	    snprintf(str, sizeof(str), "Invalid issuerDN %s", flow->protos.tls_quic.issuerDN);
@@ -587,7 +587,7 @@ static void processCertificateElements(struct ndpi_detection_module_struct *ndpi
 		      We cannot use ndpi_is_valid_hostname() as we can have wildcards
 		      here that will create false positives
 		    */
-		    if(ndpi_is_printable_string(dNSName, dNSName_len) == 0) {
+		    if(ndpi_normalize_printable_string(dNSName, dNSName_len) == 0) {
 		      ndpi_set_risk(ndpi_struct, flow, NDPI_INVALID_CHARACTERS, dNSName);
 
 		      /* This looks like an attack */
@@ -1531,7 +1531,7 @@ int processClientServerHello(struct ndpi_detection_module_struct *ndpi_struct,
 #ifdef DEBUG_TLS
 	  printf("Server TLS [ALPN: %s][len: %u]\n", alpn_str, alpn_str_len);
 #endif
-	  if(ndpi_is_printable_string(alpn_str, alpn_str_len) == 0)
+	  if(ndpi_normalize_printable_string(alpn_str, alpn_str_len) == 0)
 	    ndpi_set_risk(ndpi_struct, flow, NDPI_INVALID_CHARACTERS, alpn_str);
 
 	  if(flow->protos.tls_quic.alpn == NULL)