]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
Merge pull request #518 in SNORT/snort3 from appid_ws2_plugins to master
authorRuss Combs (rucombs) <rucombs@cisco.com>
Wed, 15 Jun 2016 23:23:32 +0000 (19:23 -0400)
committerRuss Combs (rucombs) <rucombs@cisco.com>
Wed, 15 Jun 2016 23:23:32 +0000 (19:23 -0400)
Squashed commit of the following:

commit ef92a3c83ddd737d29f044c2d8f70098a81f8574
Merge: f05eab5 a77a7f0
Author: davis mcpherson <davis.mcpherson@gmail.com>
Date:   Wed Jun 15 13:10:13 2016 -0400

    Merge branch 'appid_ws2_plugins' of ssh://bitbucket-eng-rtp1.cisco.com:7999/snort/snort3 into appid_ws2_plugins

commit f05eab50c2d9853be3e73d7cc39fa469e167d8b9
Author: davis mcpherson <davis.mcpherson@gmail.com>
Date:   Wed Jun 15 13:10:07 2016 -0400

    fix issues identified by static analysis, mostly vars unused after assignment

commit a77a7f045c86c280188f49abe65ac065887cd1e9
Author: Steve Chew <stechew@cisco.com>
Date:   Wed Jun 15 11:44:44 2016 -0400

    Added stats for ftp and telnet service plugins.

commit 2fa8f24420d27c3fecbc0b8c37109fbacbe2d8d1
Author: Steve Chew <stechew@cisco.com>
Date:   Wed Jun 15 10:48:41 2016 -0400

    Added counts for ftp and telnet.

14 files changed:
src/network_inspectors/appid/appid_flow_data.cc
src/network_inspectors/appid/appid_module.cc
src/network_inspectors/appid/appid_module.h
src/network_inspectors/appid/client_plugins/client_app_base.cc
src/network_inspectors/appid/client_plugins/client_app_smtp.cc
src/network_inspectors/appid/detector_plugins/detector_http.cc
src/network_inspectors/appid/detector_plugins/http_url_patterns.cc
src/network_inspectors/appid/fw_appid.cc
src/network_inspectors/appid/lua_detector_api.cc
src/network_inspectors/appid/service_plugins/service_base.cc
src/network_inspectors/appid/service_plugins/service_ftp.cc
src/network_inspectors/appid/service_plugins/service_mdns.cc
src/network_inspectors/appid/service_plugins/service_ssl.cc
src/network_inspectors/appid/service_plugins/service_telnet.cc

index 7bac93c2e060eec531baee00db473b723d632835..740c1b64b724e332970636c034245a4cb29bfd28 100644 (file)
@@ -375,8 +375,7 @@ AppIdData* AppIdEarlySessionCreate(
     }
 
     data = appSharedDataAlloc(proto, cliIp);
-    if (data)
-        data->common.policyId = appIdPolicyId;
+    data->common.policyId = appIdPolicyId;
 
     // FIXIT - expect session control packet support not ported to snort3 yet
     //node = (flags & APPID_EARLY_SESSION_FLAG_FW_RULE) ? &ctrlPkt->expectedSession : nullptr;
index e0d2dd95df25e2e32449dd5dd418a7dce138303f..6af1386a4996cc84cc0ad35e5c737e5f61485417 100644 (file)
@@ -36,10 +36,13 @@ THREAD_LOCAL ProfileStats appidPerfStats;
 const PegInfo appid_pegs[] =
 {
     { "packet_count", "count of packets processed by appid" },
-    { "dns_udp_count", "count of dns flows over udp discovered" },
-    { "dns_tcp_count", "count of dns flows over tcp discovered" },
-    { "smtp_count", "count of smtp flows discovered" },
-    { "smtps_count", "count of smtps flows discovered" },
+    { "dns_udp_count", "count of dns flows over udp discovered by appid" },
+    { "dns_tcp_count", "count of dns flows over tcp discovered by appid" },
+    { "ftp_count", "count of ftp flows discovered by appid" },
+    { "ftps_count", "count of ftps flows discovered by appid" },
+    { "smtp_count", "count of smtp flows discovered by appid" },
+    { "smtps_count", "count of smtps flows discovered by appid" },
+    { "telnet_count", "count of telnet flows discovered by appid" },
     { nullptr, nullptr }
 };
 
index fe49cd981313a9e5c4c9978eb1739fa9f676d485..dc4ffbb39b3601cf63cf374a3f37703263bd02e4 100644 (file)
@@ -40,8 +40,11 @@ struct AppIdStats
     PegCount packet_count;
     PegCount dns_udp_count;
     PegCount dns_tcp_count;
+    PegCount ftp_count;
+    PegCount ftps_count;
     PegCount smtp_count;
     PegCount smtps_count;
+    PegCount telnet_count;
 };
 
 extern THREAD_LOCAL AppIdStats appid_stats;
index 05ea6a717a70166c6b22681d3c44f92d542ebeb8..02657a24dd9b9716197f4b167c763cf952fe08e8 100644 (file)
@@ -495,10 +495,6 @@ static void DisplayClientAppConfig(ClientAppConfig* config)
     }
     tmp = snprintf(&buffer[position], MAX_DISPLAY_SIZE-position,
         "----------------------------------------------\n");
-    if (tmp >= MAX_DISPLAY_SIZE-position)
-        position = MAX_DISPLAY_SIZE;
-    else if (tmp > 0)
-        position += tmp;
 
     DebugFormat(DEBUG_LOG,"%s\n",buffer);
 }
index a9dc1bed8737ce24b61c1add8e6bc72d06446631..c393c9717b532c9b6601aa3f916e87d1ead836b0 100644 (file)
@@ -488,10 +488,8 @@ static CLIENT_APP_RETCODE smtp_validate(const uint8_t* data, uint16_t size, cons
 {
     ClientSMTPData* fd;
     const uint8_t* end;
-    unsigned len;
 #if UNIT_TESTING
     SMTPState currState = SMTP_STATE_NONE;
-
 #endif
 
     fd = (ClientSMTPData*)smtp_client_mod.api->data_get(flowp, smtp_client_mod.flow_data_index);
@@ -548,7 +546,6 @@ static CLIENT_APP_RETCODE smtp_validate(const uint8_t* data, uint16_t size, cons
         switch (fd->state)
         {
         case SMTP_STATE_HELO:
-            len = end - data;
             if (*data == HELO[fd->pos])
             {
                 fd->pos++;
@@ -619,6 +616,7 @@ static CLIENT_APP_RETCODE smtp_validate(const uint8_t* data, uint16_t size, cons
             else
                 goto done;
             break;
+
         case SMTP_STATE_RCPT_TO:
             if (*data == RCPTTO[fd->pos])
             {
@@ -656,10 +654,11 @@ static CLIENT_APP_RETCODE smtp_validate(const uint8_t* data, uint16_t size, cons
                 }
             }
             break;
+
         case SMTP_STATE_MESSAGE:
             if (*data == '.')
             {
-                len = end - data;
+                unsigned len = end - data;
                 if (len == 0 ||
                     (len >= 1 && data[1] == 0x0A) ||
                     (len >= 2 && data[1] == 0x0D && data[2] == 0x0A))
index b23d86f09abc6588c07a1e62216bac14b8d0a35f..9c58961127a003c084d9496db5086af5fdf533af 100644 (file)
@@ -2030,10 +2030,9 @@ int getHTTPHeaderLocation(const uint8_t* data, unsigned size, HttpId id, int* st
     return 0;
 }
 
-AppId getAppIdFromUrl(char* host, char* url, char** version,
-    char* referer, AppId* ClientAppId, AppId* serviceAppId,
-    AppId* payloadAppId, AppId* referredPayloadAppId, unsigned from_rtmp,
-    const DetectorHttpConfig* pHttpConfig)
+AppId getAppIdFromUrl(char* host, char* url, char** version, char* referer, AppId* ClientAppId,
+        AppId* serviceAppId, AppId* payloadAppId, AppId* referredPayloadAppId,
+        unsigned from_rtmp, const DetectorHttpConfig* pHttpConfig)
 {
     char* path;
     char* referer_start;
@@ -2077,20 +2076,14 @@ AppId getAppIdFromUrl(char* host, char* url, char** version,
         url_len = strlen(url);
     }
     else
-    {
         url_len = 0;
-    }
 
     if (!host)
     {
-        host_len = url_len;
-
         temp_host = host = snort_strdup(url);
         host  = strchr(host, '/');
         if (host != nullptr)
-        {
             *host = '\0';
-        }
         host = temp_host;
     }
     host_len = strlen(host);
index 32dccd2f87a3cb751dbd3ec94e80a04893062dfd..db6a4f8faf2bcd4601f80e6716c2546fe764da9e 100644 (file)
@@ -204,7 +204,8 @@ int matchQueryElements(
     size_t appVersionSize
     )
 {
-    const uint8_t* index, * endKey;
+    const uint8_t* index;
+    const uint8_t* endKey;
     const uint8_t* queryEnd;
     uint32_t extractedSize;
     uint32_t copySize = 0;
@@ -217,25 +218,21 @@ int matchQueryElements(
     if (!userPattern->pattern || !packetData->pattern)
         return 0;
 
-    /*queryEnd is 1 past the end. */
+    // queryEnd is 1 past the end.  key1=value1&key2=value2
     queryEnd = packetData->pattern + packetData->patternSize;
-    index = packetData->pattern;
-    endKey = queryEnd;
-
-    /*?key1=value1&key2=value2 */
-    for (index = packetData->pattern; index < queryEnd; index = endKey+1)
+    for (index = packetData->pattern; index < queryEnd; index = endKey + 1)
     {
         /*find end of query tuple */
         endKey = (const uint8_t*)memchr (index, '&',  queryEnd - index);
         if (!endKey)
             endKey = queryEnd;
 
-        if (userPattern->patternSize < (uint32_t)(endKey-index))
+        if (userPattern->patternSize < (uint32_t)(endKey - index))
         {
             if (memcmp(index, userPattern->pattern, userPattern->patternSize) == 0)
             {
                 index += userPattern->patternSize;
-                extractedSize = (endKey-index);
+                extractedSize = (endKey - index);
                 appVersionSize--;
                 copySize = (extractedSize < appVersionSize) ? extractedSize : appVersionSize;
                 memcpy(appVersion, index, copySize);
index 8c57ce0078faa1282e3460eec73ae74427f1c901..e4cef9c5395b1361bfef5afe4f961c22d017e4e6 100644 (file)
@@ -142,9 +142,6 @@ AppIdData* appSharedDataAlloc(IpProtocol proto, const sfip_t* ip)
     //     data->reset();
     // }
 
-    if ( !data )
-        FatalError("Could not allocate AppIdData data");
-
     if (thirdparty_appid_module)
         if (!(data->tpsession = thirdparty_appid_module->session_create()))
             FatalError("Could not allocate AppIdData->tpsession data");
@@ -536,11 +533,7 @@ static inline void appIdDebugParse(const char* desc, const uint8_t* data, uint32
             break;
 
         if (length >= sizeof(info->dport))
-        {
             memcpy(&info->dport, data, sizeof(info->dport));
-            length -= sizeof(info->dport);
-            data += sizeof(info->dport);
-        }
         else
             break;
     }
@@ -2282,6 +2275,9 @@ void fwAppIdSearch(Packet* p)
         else
             protocol = IpProtocol::UDP;
 
+        // FIXIT-H: sfip_fast_equals_raw is macro that is defined as empty
+        // this cause static analysis to think ip is never used after being set, but it will be
+        // when sfip_fast_equals_raw is implemented here
         ip = p->ptrs.ip_api.get_src();
         if (session->common.initiator_port)
             direction = (session->common.initiator_port == p->ptrs.sp) ? APP_ID_FROM_INITIATOR :
index 3d20ecadda15daab476a31a1eb1d6489e0c35e17..212452f6fc469181f89c8e5bd808a5ac21be85f9 100644 (file)
@@ -1006,7 +1006,6 @@ CLIENT_APP_RETCODE validateAnyClientApp(
     {
         ErrorMessage("client %s:  validator returned non-numeric value\n",clientName);
         detector->validateParams.pkt = nullptr;
-        retValue = SERVICE_ENULL;
     }
 
     retValue = lua_tonumber(myLuaState, -1);
index b46d6457c98a6034b5c8aac7c5720bead0ab27d1..c8edc5095f9d4cbc79d2447ae2274b85cc5d92d4 100644 (file)
@@ -176,7 +176,9 @@ static RNAServiceValidationModule* static_service_list[] =
     &dns_service_mod,
 #ifdef REMOVED_WHILE_NOT_IN_USE
     &flap_service_mod,
+#endif
     &ftp_service_mod,
+#ifdef REMOVED_WHILE_NOT_IN_USE
     &irc_service_mod,
     &lpr_service_mod,
     &mysql_service_mod,
@@ -197,7 +199,9 @@ static RNAServiceValidationModule* static_service_list[] =
     &snmp_service_mod,
     &ssh_service_mod,
     &ssl_service_mod,
+#endif
     &telnet_service_mod,
+#ifdef REMOVED_WHILE_NOT_IN_USE
     &tftp_service_mod,
     &sip_service_mod,
     &directconnect_service_mod,
@@ -758,17 +762,12 @@ int serviceLoadForConfigCallback(void* symbol, AppIdConfig* pConfig)
     }
 
     svm->api = &serviceapi;
-    pp = svm->pp;
     for (pp=svm->pp; pp && pp->validate; pp++)
-    {
         if (CServiceAddPort(pp, svm, pConfig))
             return -1;
-    }
 
     if (svm->init(&svc_init_api))
-    {
         ErrorMessage("Error initializing service %s\n",svm->name);
-    }
 
     svm->next = pConfig->serviceConfig.active_service_list;
     pConfig->serviceConfig.active_service_list = svm;
@@ -816,12 +815,9 @@ int ReloadServiceModules(AppIdConfig* pConfig)
         // processing only non-lua service detectors.
         if (svm->init)
         {
-            pp = svm->pp;
             for (pp=svm->pp; pp && pp->validate; pp++)
-            {
                 if (CServiceAddPort(pp, svm, pConfig))
                     return -1;
-            }
         }
     }
 
@@ -1078,7 +1074,7 @@ static inline RNAServiceElement* AppIdGetServiceByPattern(const Packet* pkt, IpP
     patterns->find_all((char*)pkt->data, pkt->dsize, &pattern_match, false, (void*)&match_list);
 
     count = 0;
-    for (sm=match_list; sm; sm=sm->next)
+    for (sm = match_list; sm; sm = sm->next)
     {
         if (count >= smOrderedListSize)
         {
index 99bca70c36ba8b1830e8139ded38f7eb56a75ba2..b0859b4dc0db99966681d41156cf6d7bca4e4a1e 100644 (file)
@@ -32,6 +32,7 @@
 #include "application_ids.h"
 #include "service_base.h"
 #include "service_util.h"
+#include "appid_module.h"
 
 // FIXIT-H This needs to use a real SFIP function
 static SFIP_RET sfip_convert_ip_text_to_binary(const int, const char*, void*)
@@ -1294,17 +1295,20 @@ inprocess:
     case SERVICE_SUCCESS:
         if (!getAppIdFlag(flowp, APPID_SESSION_SERVICE_DETECTED))
         {
-            uint64_t encryptedFlag = getAppIdFlag(flowp, APPID_SESSION_ENCRYPTED |
-                APPID_SESSION_DECRYPTED);
+            uint64_t encryptedFlag = getAppIdFlag(flowp, 
+                APPID_SESSION_ENCRYPTED | APPID_SESSION_DECRYPTED);
+
+            // FTPS only when encrypted==1 decrypted==0
             ftp_service_mod.api->add_service(flowp, pkt, dir, &svc_element,
-                encryptedFlag == APPID_SESSION_ENCRYPTED ?                              // FTPS
-                                                                                        // only
-                                                                                        // when
-                                                                                        // encrypted==1
-                                                                                        // decrypted==0
+                encryptedFlag == APPID_SESSION_ENCRYPTED ?
                 APP_ID_FTPS : APP_ID_FTP_CONTROL,
                 fd->vendor[0] ? fd->vendor : nullptr,
                 fd->version[0] ? fd->version : nullptr, nullptr);
+
+            if(encryptedFlag == APPID_SESSION_ENCRYPTED)
+                appid_stats.ftps_count++;
+            else
+                appid_stats.ftp_count++;
         }
         return SERVICE_SUCCESS;
 
index 28876984ee6cfb0c228e0f7801e383683924a8d5..c8347f4b1ed90b0b5d4c941eba5bca258363487f 100644 (file)
@@ -288,9 +288,7 @@ static int MDNSUserAnalyser(AppIdData* flowp, const Packet* pkt, uint16_t size,
             int user_printable_index =0;
 
             if (ret_value == -1)
-            {
                 return -1;
-            }
             else if (ret_value)
             {
                 while (start_index < data_size && (!isprint(srv_original[start_index])  ||
@@ -308,9 +306,8 @@ static int MDNSUserAnalyser(AppIdData* flowp, const Packet* pkt, uint16_t size,
                 while (user_index < user_name_len)
                 {
                     if (!isprint(user_name[user_index]))
-                    {
                         return 1;
-                    }
+
                     user_index++;
                 }
 
@@ -322,9 +319,7 @@ static int MDNSUserAnalyser(AppIdData* flowp, const Packet* pkt, uint16_t size,
             if ((resp_endptr  + NEXT_MESSAGE_OFFSET  ) < (srv_original + data_size))
             {
                 data_len_str = (uint8_t*)(resp_endptr+ LENGTH_OFFSET);
-                data_len = 0;
                 data_len =  (short)( data_len_str[0]<< SHIFT_BITS | ( data_len_str[1] ));
-
                 data_size = data_size - (resp_endptr  + NEXT_MESSAGE_OFFSET + data_len -
                     srv_original);
                 /* Check if user name is available in the Domain Name field */
@@ -348,9 +343,8 @@ static int MDNSUserAnalyser(AppIdData* flowp, const Packet* pkt, uint16_t size,
                         while (user_index < user_name_len)
                         {
                             if (isprint(user_name_bkp[user_index]))
-                            {
                                 break;
-                            }
+
                             user_index++;
                         }
 
@@ -360,9 +354,8 @@ static int MDNSUserAnalyser(AppIdData* flowp, const Packet* pkt, uint16_t size,
                         while (user_printable_index < user_name_len)
                         {
                             if (!isprint(user_name_bkp [user_printable_index ]))
-                            {
                                 return 0;
-                            }
+
                             user_printable_index++;
                         }
                         /* Copy  the user name if available */
@@ -383,14 +376,10 @@ static int MDNSUserAnalyser(AppIdData* flowp, const Packet* pkt, uint16_t size,
                         return 0;
                 }
                 else
-                {
                     return 0;
-                }
             }
             else
-            {
                 return 0;
-            }
         }
     }
     else
@@ -423,7 +412,7 @@ static int MDNS_validate(ServiceValidationArgs* args)
         {
             if (pAppidActiveConfig->mod_config->mdns_user_reporting)
             {
-                ret_val = MDNSUserAnalyser(flowp, pkt, size, args->pConfig);
+                MDNSUserAnalyser(flowp, pkt, size, args->pConfig);
                 mdnsMatchListDestroy(args->pConfig);
                 goto success;
             }
index 5aff90d20f8803ddfc793c8f162c1f64431316d0..d97a855320a127008f27e10587474b6f39b224d4 100644 (file)
@@ -517,13 +517,10 @@ int parse_certificates(ServiceSSLData* ss)
                 certs_curr->common_name_ptr = (uint8_t*)start;
                 end = strstr(start, FIELD_SEPARATOR);
                 if (end)
-                {
                     length = end - start;
-                }
                 else
-                {
                     length = strlen(start);
-                }
+
                 certs_curr->common_name_len = length;
                 common_name_tot_len += length;
             }
@@ -536,21 +533,16 @@ int parse_certificates(ServiceSSLData* ss)
                 certs_curr->org_name_ptr = (uint8_t*)start;
                 end = strstr(start, FIELD_SEPARATOR);
                 if (end)
-                {
                     length = end - start;
-                }
                 else
-                {
                     length = strlen(start);
-                }
+
                 certs_curr->org_name_len = length;
                 org_name_tot_len += length;
             }
         }
         if (!success)
-        {
             goto parse_certificates_clean;
-        }
 
         // Build up concatenated string of fields.
         common_name = nullptr;
@@ -573,7 +565,7 @@ int parse_certificates(ServiceSSLData* ss)
         while (certs_curr)
         {
             /* Grab this common name. */
-            if (certs_curr->common_name_ptr && certs_curr->common_name_len)
+            if (common_name_ptr && certs_curr->common_name_ptr && certs_curr->common_name_len)
             {
                 memcpy(common_name_ptr, certs_curr->common_name_ptr, certs_curr->common_name_len);
                 common_name_ptr += certs_curr->common_name_len;
@@ -582,7 +574,7 @@ int parse_certificates(ServiceSSLData* ss)
             }
 
             /* Grab this org name. */
-            if (certs_curr->org_name_ptr && certs_curr->org_name_len)
+            if (org_name_ptr && certs_curr->org_name_ptr && certs_curr->org_name_len)
             {
                 memcpy(org_name_ptr, certs_curr->org_name_ptr, certs_curr->org_name_len);
                 org_name_ptr += certs_curr->org_name_len;
@@ -592,6 +584,7 @@ int parse_certificates(ServiceSSLData* ss)
 
             certs_curr = certs_curr->next;
         }
+
         if (common_name_tot_len)
         {
             common_name_ptr  -= 1;
index 1a54c26775da7306d8a13669de9c0018a1821953..fe50ca14850f4c786f58107c1771034518fd9175 100644 (file)
@@ -32,6 +32,7 @@
 #include "appid_flow_data.h"
 #include "application_ids.h"
 #include "service_api.h"
+#include "appid_module.h"
 
 #define TELNET_COUNT_THRESHOLD 3
 
@@ -175,6 +176,7 @@ inprocess:
 success:
     telnet_service_mod.api->add_service(flowp, args->pkt, args->dir, &svc_element,
         APP_ID_TELNET, nullptr, nullptr, nullptr);
+    appid_stats.telnet_count++;
     return SERVICE_SUCCESS;
 
 fail: