From: davis mcpherson Date: Thu, 3 Sep 2015 14:59:42 +0000 (-0400) Subject: clean up usage of SSN_DIR_* and FROM_CLIENT/FROM_SERVER #defs and flag fields, add... X-Git-Tag: 3.0.0-233~846^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b863957b0ff2f33088ea67ba70bedc606d6bca5;p=thirdparty%2Fsnort3.git clean up usage of SSN_DIR_* and FROM_CLIENT/FROM_SERVER #defs and flag fields, add unit tests to verify use of these flags (more needed!) --- diff --git a/src/flow/expect_cache.cc b/src/flow/expect_cache.cc index 8cab34d08..3cc52af68 100644 --- a/src/flow/expect_cache.cc +++ b/src/flow/expect_cache.cc @@ -433,12 +433,14 @@ bool ExpectCache::is_expected(Packet* p) { case SSN_DIR_BOTH: break; + case SSN_DIR_FROM_CLIENT: case SSN_DIR_FROM_SERVER: if (node->reversed_key != reversed_key) return false; break; } + return true; } diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index eea18b1fe..57d286bad 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -318,7 +318,7 @@ static bool is_bidirectional(const Flow* flow) // FIXIT-L init_roles* should take const Packet* static void init_roles_ip(Packet* p, Flow* flow) { - flow->ssn_state.direction = FROM_SENDER; + flow->ssn_state.direction = FROM_CLIENT; sfip_copy(flow->client_ip, p->ptrs.ip_api.get_src()); sfip_copy(flow->server_ip, p->ptrs.ip_api.get_dst()); } @@ -361,7 +361,7 @@ static void init_roles_tcp(Packet* p, Flow* flow) static void init_roles_udp(Packet* p, Flow* flow) { - flow->ssn_state.direction = FROM_SENDER; + flow->ssn_state.direction = FROM_CLIENT; sfip_copy(flow->client_ip, p->ptrs.ip_api.get_src()); flow->client_port = ntohs(p->ptrs.udph->uh_sport); sfip_copy(flow->server_ip, p->ptrs.ip_api.get_dst()); diff --git a/src/main/snort.cc b/src/main/snort.cc index ee87fa6a4..239ef4658 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -774,7 +774,7 @@ static DAQ_Verdict update_verdict(DAQ_Verdict verdict, int& inject) } } else if ( (s_packet->packet_flags & PKT_IGNORE) || - (stream.get_ignore_direction(s_packet->flow) == SSN_DIR_BOTH) ) + (stream.get_ignore_direction(s_packet->flow) == SSN_DIR_BOTH) ) { if ( !Active::get_tunnel_bypass() ) { diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc index ad0b2892d..c9b9c3d33 100644 --- a/src/network_inspectors/binder/binder.cc +++ b/src/network_inspectors/binder/binder.cc @@ -205,7 +205,7 @@ static void set_session(Flow* flow) static void set_service(Flow* flow, const HostAttributeEntry* host) { - stream.set_application_protocol_id_from_host_entry(flow, host, SSN_DIR_FROM_SERVER); + stream.set_application_protocol_id_from_host_entry(flow, host, FROM_SERVER); } static Inspector* get_gadget(Flow* flow) diff --git a/src/protocols/packet_manager.cc b/src/protocols/packet_manager.cc index 345b9ed4d..e8dd34528 100644 --- a/src/protocols/packet_manager.cc +++ b/src/protocols/packet_manager.cc @@ -387,9 +387,9 @@ static inline uint8_t GetTTL(const Packet* const p, bool forward) return 0; if ( p->packet_flags & PKT_FROM_CLIENT ) - dir = forward ? SSN_DIR_FROM_CLIENT : SSN_DIR_FROM_SERVER; + dir = forward ? FROM_CLIENT : FROM_SERVER; else - dir = forward ? SSN_DIR_FROM_SERVER : SSN_DIR_FROM_CLIENT; + dir = forward ? FROM_SERVER : FROM_CLIENT; // outermost ip is considered to be outer here, // even if it is the only ip layer ... diff --git a/src/service_inspectors/ssl/ssl_inspector.cc b/src/service_inspectors/ssl/ssl_inspector.cc index a794d2884..d4a357295 100644 --- a/src/service_inspectors/ssl/ssl_inspector.cc +++ b/src/service_inspectors/ssl/ssl_inspector.cc @@ -218,8 +218,7 @@ static inline uint32_t SSLPP_process_app(SSL_PROTO_CONF* config, uint32_t ssn_fl if (!config->max_heartbeat_len) { DebugMessage(DEBUG_SSL, "STOPPING INSPECTION (process_app)\n"); - stream.stop_inspection(packet->flow, - packet, SSN_DIR_BOTH, -1, 0); + stream.stop_inspection(packet->flow, packet, SSN_DIR_BOTH, -1, 0); counts.stopped++; } else if (!(new_flags & SSL_HEARTBEAT_SEEN)) @@ -247,8 +246,7 @@ static inline void SSLPP_process_other(SSL_PROTO_CONF* config, SSLData* sd, uint if (!config->max_heartbeat_len) { DebugMessage(DEBUG_SSL, "STOPPING INSPECTION (process_other)\n"); - stream.stop_inspection(packet->flow, - packet, SSN_DIR_BOTH, -1, 0); + stream.stop_inspection(packet->flow, packet, SSN_DIR_BOTH, -1, 0); } else if (!(new_flags & SSL_HEARTBEAT_SEEN)) { diff --git a/src/stream/icmp/icmp_session.cc b/src/stream/icmp/icmp_session.cc index 5c4168a1f..0b276e49e 100644 --- a/src/stream/icmp/icmp_session.cc +++ b/src/stream/icmp/icmp_session.cc @@ -248,7 +248,7 @@ void IcmpSession::update_direction(char dir, const sfip_t* ip, uint16_t) { if (sfip_equals(&icmp_sender_ip, ip)) { - if ((dir == SSN_DIR_FROM_SENDER) && (flow->ssn_state.direction == SSN_DIR_FROM_SENDER)) + if ((dir == SSN_DIR_FROM_CLIENT) && (flow->ssn_state.direction == FROM_CLIENT)) { /* Direction already set as SENDER */ return; @@ -256,8 +256,7 @@ void IcmpSession::update_direction(char dir, const sfip_t* ip, uint16_t) } else if (sfip_equals(&icmp_responder_ip, ip)) { - if ((dir == SSN_DIR_FROM_RESPONDER) && - (flow->ssn_state.direction == SSN_DIR_FROM_RESPONDER)) + if ((dir == SSN_DIR_FROM_SERVER) && (flow->ssn_state.direction == FROM_SERVER)) { /* Direction already set as RESPONDER */ return; diff --git a/src/stream/libtcp/stream_tcp_unit_test.cc b/src/stream/libtcp/stream_tcp_unit_test.cc index 39faf9999..1160627b9 100644 --- a/src/stream/libtcp/stream_tcp_unit_test.cc +++ b/src/stream/libtcp/stream_tcp_unit_test.cc @@ -61,7 +61,7 @@ Packet *get_syn_packet( Flow* flow ) pkt->pkt = cooked_syn; pkt->ptrs.tcph = ( tcp::TCPHdr * ) ( cooked_syn + 34 ); pkt->dsize = 0; - pkt->packet_flags &= PKT_FROM_CLIENT; + pkt->packet_flags = PKT_FROM_CLIENT; return pkt; } @@ -74,7 +74,7 @@ Packet *get_syn_ack_packet( Flow* flow ) pkt->pkt = cooked_syn_ack; pkt->ptrs.tcph = ( tcp::TCPHdr * ) ( cooked_syn_ack + 34 ); pkt->dsize = 0; - pkt->packet_flags &= PKT_FROM_SERVER; + pkt->packet_flags = PKT_FROM_SERVER; return pkt; } @@ -87,7 +87,7 @@ Packet *get_ack_packet( Flow* flow ) pkt->pkt = cooked_ack; pkt->ptrs.tcph = ( tcp::TCPHdr * ) ( cooked_ack + 34 ); pkt->dsize = 0; - pkt->packet_flags &= PKT_FROM_CLIENT; + pkt->packet_flags = PKT_FROM_CLIENT; return pkt; } @@ -100,7 +100,7 @@ Packet *get_fin_packet( Flow* flow ) pkt->pkt = cooked_fin; pkt->ptrs.tcph = ( tcp::TCPHdr * ) ( cooked_fin + 34 ); pkt->dsize = 0; - pkt->packet_flags &= PKT_FROM_CLIENT; + pkt->packet_flags = PKT_FROM_CLIENT; return pkt; } @@ -113,7 +113,7 @@ Packet *get_rst_packet( Flow* flow ) pkt->pkt = cooked_rst; pkt->ptrs.tcph = ( tcp::TCPHdr * ) ( cooked_rst + 34 ); pkt->dsize = 0; - pkt->packet_flags &= PKT_FROM_CLIENT; + pkt->packet_flags = PKT_FROM_CLIENT; return pkt; } @@ -126,7 +126,7 @@ Packet *get_data_packet( Flow* flow ) pkt->pkt = cooked_data; pkt->ptrs.tcph = ( tcp::TCPHdr * ) ( cooked_data + 34 ); pkt->dsize = 42; - pkt->packet_flags &= PKT_FROM_CLIENT; + pkt->packet_flags = PKT_FROM_CLIENT; return pkt; } diff --git a/src/stream/stream.h b/src/stream/stream.h index 2c190a156..0b2d81cb1 100644 --- a/src/stream/stream.h +++ b/src/stream/stream.h @@ -47,12 +47,6 @@ #define STREAM_CONFIG_IPS 0x00000400 #define STREAM_CONFIG_NO_ASYNC_REASSEMBLY 0x00000800 -/* traffic direction identification */ -#define FROM_SERVER 0 -#define FROM_RESPONDER 0 -#define FROM_CLIENT 1 -#define FROM_SENDER 1 - // FIXIT-L some of this stuff can be better encapsulated struct StreamGlobalConfig diff --git a/src/stream/stream_api.cc b/src/stream/stream_api.cc index 3e6783d8a..e614ac781 100644 --- a/src/stream/stream_api.cc +++ b/src/stream/stream_api.cc @@ -54,6 +54,11 @@ #include "target_based/snort_protocols.h" #include "target_based/sftarget_hostentry.h" +#ifdef UNIT_TEST +#include "test/catch.hpp" +#include "stream/libtcp/stream_tcp_unit_test.h" +#endif + Stream stream; // FIXIT-L global for SnortContext Stream::Stream() @@ -430,7 +435,7 @@ void Stream::set_application_protocol_id_from_host_entry( #if 0 // FIXIT - from client doesn't imply need to swap - if (direction == SSN_DIR_FROM_CLIENT) + if (direction == FROM_CLIENT) { if ( application_protocol && (flow->ssn_state.session_flags & SSNFLAG_MIDSTREAM) ) @@ -469,7 +474,7 @@ int16_t Stream::get_application_protocol_id(Flow* flow) host_entry = SFAT_LookupHostEntryByIP(&flow->server_ip); if (host_entry) { - set_application_protocol_id_from_host_entry(flow, host_entry, SSN_DIR_FROM_SERVER); + set_application_protocol_id_from_host_entry(flow, host_entry, FROM_SERVER); if (flow->ssn_state.application_protocol != 0) { @@ -478,10 +483,9 @@ int16_t Stream::get_application_protocol_id(Flow* flow) } host_entry = SFAT_LookupHostEntryByIP(&flow->client_ip); - if (host_entry) { - set_application_protocol_id_from_host_entry(flow, host_entry, SSN_DIR_FROM_CLIENT); + set_application_protocol_id_from_host_entry(flow, host_entry, FROM_CLIENT); if (flow->ssn_state.application_protocol != 0) { @@ -596,7 +600,7 @@ uint8_t Stream::get_session_ttl(Flow* flow, char dir, bool outer) if ( !flow ) return 0; - if ( SSN_DIR_FROM_CLIENT == dir ) + if ( FROM_CLIENT == dir ) return outer ? flow->outer_client_ttl : flow->inner_client_ttl; return outer ? flow->outer_server_ttl : flow->inner_server_ttl; @@ -660,10 +664,8 @@ bool Stream::blocked_session(Flow* flow, Packet* p) bool Stream::ignored_session(Flow* flow, Packet* p) { - if ( - ((p->packet_flags & PKT_FROM_SERVER) && + if (((p->packet_flags & PKT_FROM_SERVER) && (flow->ssn_state.ignore_direction & SSN_DIR_FROM_CLIENT)) || - ((p->packet_flags & PKT_FROM_CLIENT) && (flow->ssn_state.ignore_direction & SSN_DIR_FROM_SERVER)) ) { @@ -839,3 +841,143 @@ bool Stream::missed_packets(Flow* flow, uint8_t dir) return flow->session->are_packets_missing(dir); } + +#ifdef UNIT_TEST + +#include "framework/cursor.h" + +TEST_CASE("Stream API", "[stream_api][stream]") +{ + // initialization code here + Flow* flow = new Flow; + + SECTION("set/get ignore direction") + { + int dir = Stream::set_ignore_direction( flow, SSN_DIR_NONE); + CHECK( ( dir == SSN_DIR_NONE ) ); + dir = Stream::get_ignore_direction( flow ); + CHECK( ( dir == SSN_DIR_NONE ) ); + + dir = Stream::set_ignore_direction( flow, SSN_DIR_FROM_CLIENT); + CHECK( ( dir == SSN_DIR_FROM_CLIENT ) ); + dir = Stream::get_ignore_direction( flow ); + CHECK( ( dir == SSN_DIR_FROM_CLIENT ) ); + + dir = Stream::set_ignore_direction( flow, SSN_DIR_FROM_SERVER); + CHECK( ( dir == SSN_DIR_FROM_SERVER ) ); + dir = Stream::get_ignore_direction( flow ); + CHECK( ( dir == SSN_DIR_FROM_SERVER ) ); + + dir = Stream::set_ignore_direction( flow, SSN_DIR_BOTH); + CHECK( ( dir == SSN_DIR_BOTH ) ); + dir = Stream::get_ignore_direction( flow ); + CHECK( ( dir == SSN_DIR_BOTH ) ); + } + + SECTION("stop inspection") + { + Packet* pkt = get_syn_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_FROM_CLIENT, 0, 0 ); + dir = Stream::get_ignore_direction( flow ); + CHECK( ( dir == SSN_DIR_FROM_CLIENT ) ); + CHECK( ( flow->flow_state == Flow::ALLOW ) ); + + Stream::stop_inspection( flow, pkt, SSN_DIR_FROM_SERVER, 0, 0 ); + dir = Stream::get_ignore_direction( flow ); + CHECK( ( dir == SSN_DIR_FROM_SERVER ) ); + CHECK( ( flow->flow_state == Flow::ALLOW ) ); + + delete pkt->flow->session; + delete pkt; + } + + SECTION("stop inspection from server - client packet") + { + Packet* pkt = get_syn_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_FROM_SERVER, 0, 0 ); + bool ignored = Stream::ignored_session( flow, pkt ); + CHECK( ignored ); + + delete pkt->flow->session; + delete pkt; + } + + SECTION("stop inspection from server - server packet") + { + Packet* pkt = get_syn_ack_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_FROM_SERVER, 0, 0 ); + bool ignored = Stream::ignored_session( flow, pkt ); + CHECK( !ignored ); + delete pkt->flow->session; + delete pkt; + } + + SECTION("stop inspection from client - client packet") + { + Packet* pkt = get_syn_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_FROM_CLIENT, 0, 0 ); + bool ignored = Stream::ignored_session( flow, pkt ); + CHECK( !ignored ); + + delete pkt->flow->session; + delete pkt; + } + + SECTION("stop inspection from client - server packet") + { + Packet* pkt = get_syn_ack_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_FROM_CLIENT, 0, 0 ); + bool ignored = Stream::ignored_session( flow, pkt ); + CHECK( ignored ); + delete pkt->flow->session; + delete pkt; + } + + SECTION("stop inspection both - client packet") + { + Packet* pkt = get_syn_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_BOTH, 0, 0 ); + bool ignored = Stream::ignored_session( flow, pkt ); + CHECK( ignored ); + + delete pkt->flow->session; + delete pkt; + } + + SECTION("stop inspection both - server packet") + { + Packet* pkt = get_syn_ack_packet( flow ); + pkt->flow->session = new TcpSession( flow ); + int dir; + + Stream::stop_inspection( flow, pkt, SSN_DIR_BOTH, 0, 0 ); + bool ignored = Stream::ignored_session( flow, pkt ); + CHECK( ignored ); + delete pkt->flow->session; + delete pkt; + } + + delete flow; +} + + +#endif + diff --git a/src/stream/stream_api.h b/src/stream/stream_api.h index fd7defa83..750b9a5b4 100644 --- a/src/stream/stream_api.h +++ b/src/stream/stream_api.h @@ -35,6 +35,10 @@ #include "flow/flow.h" #include "main/snort_types.h" +/* traffic direction identification */ +#define FROM_SERVER 0 +#define FROM_CLIENT 1 + #define SSN_MISSING_NONE 0x00 #define SSN_MISSING_BEFORE 0x01 #define SSN_MISSING_AFTER 0x02 @@ -42,9 +46,7 @@ #define SSN_DIR_NONE 0x00 #define SSN_DIR_FROM_CLIENT 0x01 -#define SSN_DIR_FROM_SENDER 0x01 #define SSN_DIR_FROM_SERVER 0x02 -#define SSN_DIR_FROM_RESPONDER 0x02 #define SSN_DIR_BOTH 0x03 // sequence must match FRAG_POLICY_* enum in stream_ip.h (1-based) diff --git a/src/stream/tcp/tcp_reassembly.cc b/src/stream/tcp/tcp_reassembly.cc index 51f795259..8ed3013c3 100644 --- a/src/stream/tcp/tcp_reassembly.cc +++ b/src/stream/tcp/tcp_reassembly.cc @@ -1442,15 +1442,16 @@ bool StreamGetReassemblyFlushPolicyTcp(Flow *flow, char dir) tcpssn = (TcpSession*) flow->session; - if (dir & SSN_DIR_FROM_CLIENT) + if (dir & FROM_CLIENT) { return (char) tcpssn->client.flush_policy != STREAM_FLPOLICY_IGNORE; } - if (dir & SSN_DIR_FROM_SERVER) + if (dir & FROM_SERVER) { return (char) tcpssn->server.flush_policy != STREAM_FLPOLICY_IGNORE; } + return false; } diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc index 5a564624f..eaf22cd1d 100644 --- a/src/stream/tcp/tcp_session.cc +++ b/src/stream/tcp/tcp_session.cc @@ -1178,7 +1178,8 @@ static inline void CopyMacAddr(Packet* p, TcpSession* tcpssn, int dir) tcpssn->client.mac_addr[i] = eh->ether_src[i]; tcpssn->server.mac_addr[i] = eh->ether_dst[i]; } - } else + } + else { /* Server is SRC */ for (i = 0; i < 6; i++) @@ -2904,7 +2905,8 @@ uint8_t TcpSession::missing_in_reassembled(uint8_t dir) return SSN_MISSING_BEFORE; else if (server.flags & TF_MISSING_PKT) return SSN_MISSING_AFTER; - } else if (dir & SSN_DIR_FROM_SERVER) + } + else if (dir & SSN_DIR_FROM_SERVER) { if ((client.flags & TF_MISSING_PKT) && (client.flags & TF_MISSING_PREV_PKT)) @@ -2943,8 +2945,7 @@ void TcpSession::update_direction(char dir, const sfip_t* ip, uint16_t port) if (sfip_equals(&flow->client_ip, ip) && (flow->client_port == port)) { - if ((dir == SSN_DIR_FROM_CLIENT) - && (flow->ssn_state.direction == SSN_DIR_FROM_CLIENT)) + if ((dir == SSN_DIR_FROM_CLIENT) && (flow->ssn_state.direction == FROM_CLIENT)) { /* Direction already set as client */ return; @@ -2952,8 +2953,7 @@ void TcpSession::update_direction(char dir, const sfip_t* ip, uint16_t port) } else if (sfip_equals(&flow->server_ip, ip) && (flow->server_port == port)) { - if ((dir == SSN_DIR_FROM_SERVER) - && (flow->ssn_state.direction == SSN_DIR_FROM_SERVER)) + if ((dir == SSN_DIR_FROM_SERVER) && (flow->ssn_state.direction == FROM_SERVER)) { /* Direction already set as server */ return; diff --git a/src/stream/udp/udp_session.cc b/src/stream/udp/udp_session.cc index 63714135b..d583e8a41 100644 --- a/src/stream/udp/udp_session.cc +++ b/src/stream/udp/udp_session.cc @@ -145,7 +145,7 @@ bool UdpSession::setup(Packet* p) flow->ssn_state.session_flags |= SSNFLAG_SEEN_SENDER; flow->protocol = p->type(); - flow->ssn_state.direction = FROM_SENDER; + flow->ssn_state.direction = FROM_CLIENT; StreamUdpConfig* pc = get_udp_cfg(flow->ssn_server); flow->set_expire(p, pc->session_timeout); @@ -154,10 +154,7 @@ bool UdpSession::setup(Packet* p) AddUDPSession(&sfBase); if (perfmon_config && (perfmon_config->perf_flags & SFPERF_FLOWIP)) - { - UpdateFlowIPState(&sfFlow, &flow->client_ip, - &flow->server_ip, SFS_STATE_UDP_CREATED); - } + UpdateFlowIPState(&sfFlow, &flow->client_ip, &flow->server_ip, SFS_STATE_UDP_CREATED); if ( flow_con->expected_flow(flow, p) ) return false; @@ -180,18 +177,17 @@ void UdpSession::update_direction( if (sfip_equals(&flow->client_ip, ip) && (flow->client_port == port)) { - if ((dir == SSN_DIR_FROM_SENDER) && (flow->ssn_state.direction == SSN_DIR_FROM_SENDER)) + if ((dir == SSN_DIR_FROM_CLIENT) && (flow->ssn_state.direction == FROM_CLIENT)) { - /* Direction already set as SENDER */ + /* Direction already set as CLIENT */ return; } } else if (sfip_equals(&flow->server_ip, ip) && (flow->server_port == port)) { - if ((dir == SSN_DIR_FROM_RESPONDER) && - (flow->ssn_state.direction == SSN_DIR_FROM_RESPONDER)) + if ((dir == SSN_DIR_FROM_SERVER) && (flow->ssn_state.direction == FROM_SERVER)) { - /* Direction already set as RESPONDER */ + /* Direction already set as SERVER */ return; } }