From 206c52201a96434a5a5049c3a2f942ae504ef1a6 Mon Sep 17 00:00:00 2001 From: "Russ Combs (rucombs)" Date: Wed, 15 Jun 2016 19:23:32 -0400 Subject: [PATCH] Merge pull request #518 in SNORT/snort3 from appid_ws2_plugins to master Squashed commit of the following: commit ef92a3c83ddd737d29f044c2d8f70098a81f8574 Merge: f05eab5 a77a7f0 Author: davis mcpherson 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 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 Date: Wed Jun 15 11:44:44 2016 -0400 Added stats for ftp and telnet service plugins. commit 2fa8f24420d27c3fecbc0b8c37109fbacbe2d8d1 Author: Steve Chew Date: Wed Jun 15 10:48:41 2016 -0400 Added counts for ftp and telnet. --- .../appid/appid_flow_data.cc | 3 +-- src/network_inspectors/appid/appid_module.cc | 11 +++++++---- src/network_inspectors/appid/appid_module.h | 3 +++ .../appid/client_plugins/client_app_base.cc | 4 ---- .../appid/client_plugins/client_app_smtp.cc | 7 +++---- .../appid/detector_plugins/detector_http.cc | 13 +++---------- .../detector_plugins/http_url_patterns.cc | 15 ++++++--------- src/network_inspectors/appid/fw_appid.cc | 10 +++------- .../appid/lua_detector_api.cc | 1 - .../appid/service_plugins/service_base.cc | 14 +++++--------- .../appid/service_plugins/service_ftp.cc | 18 +++++++++++------- .../appid/service_plugins/service_mdns.cc | 19 ++++--------------- .../appid/service_plugins/service_ssl.cc | 17 +++++------------ .../appid/service_plugins/service_telnet.cc | 2 ++ 14 files changed, 53 insertions(+), 84 deletions(-) diff --git a/src/network_inspectors/appid/appid_flow_data.cc b/src/network_inspectors/appid/appid_flow_data.cc index 7bac93c2e..740c1b64b 100644 --- a/src/network_inspectors/appid/appid_flow_data.cc +++ b/src/network_inspectors/appid/appid_flow_data.cc @@ -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; diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc index e0d2dd95d..6af1386a4 100644 --- a/src/network_inspectors/appid/appid_module.cc +++ b/src/network_inspectors/appid/appid_module.cc @@ -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 } }; diff --git a/src/network_inspectors/appid/appid_module.h b/src/network_inspectors/appid/appid_module.h index fe49cd981..dc4ffbb39 100644 --- a/src/network_inspectors/appid/appid_module.h +++ b/src/network_inspectors/appid/appid_module.h @@ -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; diff --git a/src/network_inspectors/appid/client_plugins/client_app_base.cc b/src/network_inspectors/appid/client_plugins/client_app_base.cc index 05ea6a717..02657a24d 100644 --- a/src/network_inspectors/appid/client_plugins/client_app_base.cc +++ b/src/network_inspectors/appid/client_plugins/client_app_base.cc @@ -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); } diff --git a/src/network_inspectors/appid/client_plugins/client_app_smtp.cc b/src/network_inspectors/appid/client_plugins/client_app_smtp.cc index a9dc1bed8..c393c9717 100644 --- a/src/network_inspectors/appid/client_plugins/client_app_smtp.cc +++ b/src/network_inspectors/appid/client_plugins/client_app_smtp.cc @@ -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)) diff --git a/src/network_inspectors/appid/detector_plugins/detector_http.cc b/src/network_inspectors/appid/detector_plugins/detector_http.cc index b23d86f09..9c5896112 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_http.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_http.cc @@ -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); diff --git a/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc b/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc index 32dccd2f8..db6a4f8fa 100644 --- a/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc +++ b/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc @@ -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); diff --git a/src/network_inspectors/appid/fw_appid.cc b/src/network_inspectors/appid/fw_appid.cc index 8c57ce007..e4cef9c53 100644 --- a/src/network_inspectors/appid/fw_appid.cc +++ b/src/network_inspectors/appid/fw_appid.cc @@ -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 : diff --git a/src/network_inspectors/appid/lua_detector_api.cc b/src/network_inspectors/appid/lua_detector_api.cc index 3d20ecadd..212452f6f 100644 --- a/src/network_inspectors/appid/lua_detector_api.cc +++ b/src/network_inspectors/appid/lua_detector_api.cc @@ -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); diff --git a/src/network_inspectors/appid/service_plugins/service_base.cc b/src/network_inspectors/appid/service_plugins/service_base.cc index b46d6457c..c8edc5095 100644 --- a/src/network_inspectors/appid/service_plugins/service_base.cc +++ b/src/network_inspectors/appid/service_plugins/service_base.cc @@ -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) { diff --git a/src/network_inspectors/appid/service_plugins/service_ftp.cc b/src/network_inspectors/appid/service_plugins/service_ftp.cc index 99bca70c3..b0859b4dc 100644 --- a/src/network_inspectors/appid/service_plugins/service_ftp.cc +++ b/src/network_inspectors/appid/service_plugins/service_ftp.cc @@ -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; diff --git a/src/network_inspectors/appid/service_plugins/service_mdns.cc b/src/network_inspectors/appid/service_plugins/service_mdns.cc index 28876984e..c8347f4b1 100644 --- a/src/network_inspectors/appid/service_plugins/service_mdns.cc +++ b/src/network_inspectors/appid/service_plugins/service_mdns.cc @@ -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; } diff --git a/src/network_inspectors/appid/service_plugins/service_ssl.cc b/src/network_inspectors/appid/service_plugins/service_ssl.cc index 5aff90d20..d97a85532 100644 --- a/src/network_inspectors/appid/service_plugins/service_ssl.cc +++ b/src/network_inspectors/appid/service_plugins/service_ssl.cc @@ -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; diff --git a/src/network_inspectors/appid/service_plugins/service_telnet.cc b/src/network_inspectors/appid/service_plugins/service_telnet.cc index 1a54c2677..fe50ca148 100644 --- a/src/network_inspectors/appid/service_plugins/service_telnet.cc +++ b/src/network_inspectors/appid/service_plugins/service_telnet.cc @@ -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: -- 2.47.3