From: Hui Cao (huica) Date: Fri, 28 Oct 2016 19:47:38 +0000 (-0400) Subject: Merge pull request #688 in SNORT/snort3 from appid_detector_server_packet_fix to... X-Git-Tag: 3.0.0-233~206 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ee4936b9b1a046a9b7265a4af5c96431f214b07;p=thirdparty%2Fsnort3.git Merge pull request #688 in SNORT/snort3 from appid_detector_server_packet_fix to master Squashed commit of the following: commit 984af329150fad81b371336891202176ef9543cb Author: davis mcpherson Date: Tue Oct 25 16:34:18 2016 -0400 modify 'client gets server packets' processing to clear this flag when a packet is seen by the service side validate function. if it sees one it will see them all so need need for client side to process server side packets in this case --- diff --git a/src/network_inspectors/appid/appid_api.cc b/src/network_inspectors/appid/appid_api.cc index b4e7aefde..d38c91d88 100644 --- a/src/network_inspectors/appid/appid_api.cc +++ b/src/network_inspectors/appid/appid_api.cc @@ -180,6 +180,7 @@ bool AppIdApi::is_appid_inspecting_session(AppIdSession* appIdSession) return true; } } + return false; } diff --git a/src/network_inspectors/appid/detector_plugins/detector_imap.cc b/src/network_inspectors/appid/detector_plugins/detector_imap.cc index 749a1f573..0db712f4a 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_imap.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_imap.cc @@ -402,8 +402,8 @@ static int imap_server_validate(DetectorData* dd, const uint8_t* data, uint16_t const uint8_t* end = data + size; ServiceIMAPData* id = &dd->server; - id->flags &= ~IMAP_FLAG_RESULT_ALL; // when we are done these flags will tell us OK vs. NO vs. - // BAD + id->flags &= ~IMAP_FLAG_RESULT_ALL; // flags will tell us OK vs. NO vs. BAD + for (; data < end; data++) { #ifdef DEBUG_IMAP_DETECTOR @@ -572,15 +572,10 @@ static int imap_server_validate(DetectorData* dd, const uint8_t* data, uint16_t break; case IMAP_STATE_MID_OK_LOGIN: - /*add user successful */ + // add user successful - note: use of LOGIN cmd implies no IMAPS if ((id->flags & IMAP_FLAG_RESULT_OK) && dd->client.username[0]) - { - service_mod.api->add_user(asd, dd->client.username, APP_ID_IMAP, 1); // use of - // LOGIN - // cmd - // implies - // no IMAPS - } + service_mod.api->add_user(asd, dd->client.username, APP_ID_IMAP, 1); + id->state = IMAP_STATE_MID_LINE; break; case IMAP_STATE_MID_NO: @@ -608,17 +603,9 @@ static int imap_server_validate(DetectorData* dd, const uint8_t* data, uint16_t if (id->pos >= sizeof(NO_LOGIN)-1) { id->state = IMAP_STATE_ALNUM_CODE_TERM; - /*add user login failed */ + // add user login failed - note: use of LOGIN cmd implies no IMAPS if ((id->flags & IMAP_FLAG_RESULT_NO) && dd->client.username[0]) - { - service_mod.api->add_user(asd, dd->client.username, APP_ID_IMAP, 0); // use - // of - // LOGIN - // cmd - // implies - // no - // IMAPS - } + service_mod.api->add_user(asd, dd->client.username, APP_ID_IMAP, 0); } } else @@ -657,31 +644,25 @@ static int imap_server_validate(DetectorData* dd, const uint8_t* data, uint16_t break; } } + if (dd->client.state == IMAP_CLIENT_STATE_STARTTLS_CMD) { if (id->flags & IMAP_FLAG_RESULT_OK) { + client_app_mod.api->add_app(asd, APP_ID_IMAPS, APP_ID_IMAPS, nullptr); asd->clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); - /* we are potentially overriding any APP_ID_IMAP assessment that was made earlier. */ - client_app_mod.api->add_app(asd, APP_ID_IMAPS, APP_ID_IMAPS, nullptr); // sets - // APPID_SESSION_CLIENT_DETECTED } else - { - /* We failed to transition to IMAPS - fall back to normal IMAP state, Non-Authenticated - */ dd->client.state = IMAP_CLIENT_STATE_NON_AUTH; - } } else if (dd->client.state == IMAP_CLIENT_STATE_AUTHENTICATE_CMD) { - dd->client.auth = 0; // stop discarding intervening command packets (part of the - // authenticate) - /* Change state as appropriate */ + // stop discarding intervening command packets (part of the authenticate) + dd->client.auth = 0; dd->client.state = (id->flags & IMAP_FLAG_RESULT_OK) ? - IMAP_CLIENT_STATE_AUTH : - IMAP_CLIENT_STATE_NON_AUTH; + IMAP_CLIENT_STATE_AUTH : IMAP_CLIENT_STATE_NON_AUTH; } + return 0; } @@ -878,7 +859,7 @@ static CLIENT_APP_RETCODE validate(const uint8_t* data, uint16_t size, const int { asd->set_session_flags(APPID_SESSION_CLIENT_DETECTED); asd->clear_session_flags( - APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); + APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); } } *p = 0; @@ -994,6 +975,9 @@ static int imap_validate(ServiceValidationArgs* args) else id = &dd->server; + // server side is seeing packets so no need for client side to process them + asd->clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); + if (dd->need_continue) asd->set_session_flags(APPID_SESSION_CONTINUE); else diff --git a/src/network_inspectors/appid/detector_plugins/detector_kerberos.cc b/src/network_inspectors/appid/detector_plugins/detector_kerberos.cc index 8248783f8..c52c0226d 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_kerberos.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_kerberos.cc @@ -1048,6 +1048,9 @@ static int krb_server_validate(ServiceValidationArgs* args) if (!size) goto inprocess; + // server side is seeing packets so no need for client side to process them + asd->clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); + fd = (DetectorData*)kerberos_detector_mod.api->data_get(asd, kerberos_detector_mod.flow_data_index); if (!fd) diff --git a/src/network_inspectors/appid/detector_plugins/detector_pop3.cc b/src/network_inspectors/appid/detector_plugins/detector_pop3.cc index d6ed64409..60bb00a8f 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_pop3.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_pop3.cc @@ -453,22 +453,19 @@ static int pop3_server_validate(POP3DetectorData* dd, const uint8_t* data, uint1 { if (pd->error) { - /* We failed to transition to POP3S - fall back to normal POP3 state, AUTHORIZATION - */ - dd->client.state = POP3_CLIENT_STATE_AUTH; + // We failed to transition to POP3S - fall back to normal POP3 state, AUTHORIZATION + dd->client.state = POP3_CLIENT_STATE_AUTH; } else { + // we are potentially overriding the APP_ID_POP3 assessment that was made earlier. + // sets APPID_SESSION_CLIENT_DETECTED asd->set_session_flags(APPID_SESSION_ENCRYPTED); asd->clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); - /* we are potentially overriding the APP_ID_POP3 assessment that was made earlier. - */ - client_app_mod.api->add_app(asd, APP_ID_POP3S, APP_ID_POP3S, nullptr); // sets - // APPID_SESSION_CLIENT_DETECTED + client_app_mod.api->add_app(asd, APP_ID_POP3S, APP_ID_POP3S, nullptr); } } - else if (dd->client.username) // possible only with non-TLS authentication therefor: - // APP_ID_POP3 + else if (dd->client.username) // possible only with non-TLS authentication therefore APP_ID_POP3 { if (pd->error) { @@ -850,10 +847,9 @@ static CLIENT_APP_RETCODE pop3_ca_validate(const uint8_t* data, uint16_t size, c case POP3_CLIENT_STATE_TRANS: if (pattern_index >= PATTERN_POP3_OTHER) { - /* We stayed in non-secure mode and received a TRANSACTION-state command: POP3 - found */ - client_app_mod.api->add_app(asd, APP_ID_POP3, APP_ID_POP3, nullptr); // sets - // APPID_SESSION_CLIENT_DETECTED + // Still in non-secure mode and received a TRANSACTION-state command: POP3 found + // sets APPID_SESSION_CLIENT_DETECTED + client_app_mod.api->add_app(asd, APP_ID_POP3, APP_ID_POP3, nullptr); fd->detected = 1; } else @@ -910,6 +906,9 @@ static int pop3_validate(ServiceValidationArgs* args) else pd = &dd->server; + // server side is seeing packets so no need for client side to process them + asd->clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); + if (dd->need_continue) asd->set_session_flags(APPID_SESSION_CONTINUE); else diff --git a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc index 9de1c35d2..7db16eb82 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc @@ -570,7 +570,6 @@ static inline SMTPDetectorData* smtp_get_SMTPDetectorData(AppIdSession* asd) dd->client.state = SMTP_CLIENT_STATE_HELO; dd->need_continue = 1; dd->watch_for_deprecated_port = 1; - asd->set_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); return dd; } @@ -901,6 +900,8 @@ static int smtp_svc_validate(ServiceValidationArgs* args) if (!size) goto inprocess; + asd->clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); + // Whether this is bound for the client detector or not, if client doesn't care // then clear the APPID_SESSION_CONTINUE flag and we will be done sooner. if (dd->need_continue == 0) diff --git a/src/network_inspectors/appid/test/appid_http_event_test.cc b/src/network_inspectors/appid/test/appid_http_event_test.cc index 90dd451bc..b7024b90a 100644 --- a/src/network_inspectors/appid/test/appid_http_event_test.cc +++ b/src/network_inspectors/appid/test/appid_http_event_test.cc @@ -76,78 +76,6 @@ void Field::set(int32_t length_, const uint8_t* start_) Field global_field; -const Field& HttpMsgSection::get_classic_buffer(unsigned id, uint64_t sub_id, uint64_t) -{ - global_field.set(0, nullptr); - if(id == HttpEnums::HTTP_BUFFER_HEADER) - { - if(sub_id == HttpEnums::HEAD_HOST) - { - if(host) - global_field.set(strlen(host), (const uint8_t*)host); - } - - if(sub_id == HttpEnums::HEAD_COOKIE) - { - if(cookie) - global_field.set(strlen(cookie), (const uint8_t*)cookie); - } - - if(sub_id == HttpEnums::HEAD_USER_AGENT) - { - if(useragent) - global_field.set(strlen(useragent), (const uint8_t*)useragent); - } - - if(sub_id == HttpEnums::HEAD_REFERER) - { - if(referer) - global_field.set(strlen(referer), (const uint8_t*)referer); - } - - if(sub_id == HttpEnums::HEAD_SERVER) - { - if(server) - global_field.set(strlen(server), (const uint8_t*)server); - } - - if(sub_id == HttpEnums::HEAD_X_WORKING_WITH) - { - if(x_working_with) - global_field.set(strlen(x_working_with), (const uint8_t*)x_working_with); - } - - if(sub_id == HttpEnums::HEAD_VIA) - { - if(via) - global_field.set(strlen(via), (const uint8_t*)via); - } - - if(sub_id == HttpEnums::HEAD_CONTENT_TYPE) - { - if(content_type) - global_field.set(strlen(content_type), (const uint8_t*)content_type); - } - - if(sub_id == HttpEnums::HEAD_LOCATION) - { - if(location) - global_field.set(strlen(location), (const uint8_t*)location); - } - } - - if(id == HttpEnums::HTTP_BUFFER_URI) - { - if(sub_id == 0) - { - if(uri) - global_field.set(strlen(uri), (const uint8_t*)uri); - } - } - - return global_field; -} - class FakeHttpMsgHeader { };