From: Josh Date: Wed, 13 Aug 2014 16:50:39 +0000 (-0400) Subject: fixing new packet ip_api bugs X-Git-Tag: 3.0.0-233~1426^2~8^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4496902cbb28da41eb4d354bff704b2a154832bf;p=thirdparty%2Fsnort3.git fixing new packet ip_api bugs --- diff --git a/src/codecs/ip/cd_ipv6.cc b/src/codecs/ip/cd_ipv6.cc index 24130aaf8..5ab6770df 100644 --- a/src/codecs/ip/cd_ipv6.cc +++ b/src/codecs/ip/cd_ipv6.cc @@ -142,7 +142,7 @@ bool Ipv6Codec::decode(const uint8_t *raw_pkt, const uint32_t& raw_len, /* lay the IP struct over the raw data */ ip6h = reinterpret_cast(const_cast(raw_pkt)); - if(raw_len < ipv6::hdr_len()) + if(raw_len < ipv6::IP6_HEADER_LEN) { if ((p->decode_flags & DECODE__UNSURE_ENCAP) == 0) codec_events::decoder_event(p, DECODE_IPV6_TRUNCATED); @@ -170,7 +170,7 @@ bool Ipv6Codec::decode(const uint8_t *raw_pkt, const uint32_t& raw_len, #endif - payload_len = ntohs(ip6h->ip6plen) + ipv6::hdr_len(); + payload_len = ntohs(ip6h->ip6plen) + ipv6::IP6_HEADER_LEN; if(payload_len != raw_len) { @@ -211,7 +211,7 @@ bool Ipv6Codec::decode(const uint8_t *raw_pkt, const uint32_t& raw_len, /* set the real IP length for logging */ p->proto_bits |= PROTO_BIT__IP; // extra ipv6 header will be removed in PacketManager - const_cast(raw_len) = ntohs(ip6h->get_len()) + ipv6::hdr_len(); + const_cast(raw_len) = ntohs(ip6h->get_len()) + ipv6::IP6_HEADER_LEN; // check for isatap before overwriting the ip_api. IPV6CheckIsatap(ip6h, p); @@ -222,7 +222,7 @@ bool Ipv6Codec::decode(const uint8_t *raw_pkt, const uint32_t& raw_len, CheckIPV6Multicast(ip6h, p); next_prot_id = ip6h->get_next(); - lyr_len = ipv6::hdr_len(); + lyr_len = ipv6::IP6_HEADER_LEN; return true; diff --git a/src/codecs/ip/cd_tcp.cc b/src/codecs/ip/cd_tcp.cc index 85dd47dd1..fbdffabdb 100644 --- a/src/codecs/ip/cd_tcp.cc +++ b/src/codecs/ip/cd_tcp.cc @@ -699,8 +699,9 @@ bool TcpCodec::update(Packet* p, Layer* lyr, uint32_t* len) if (p->ip_api.is_ip4()) { checksum::Pseudoheader ps; - ps.sip = ((IPHdr *)(lyr-1)->start)->ip_src; - ps.dip = ((IPHdr *)(lyr-1)->start)->ip_dst; + const ip::IPHdr* ip4h = p->ip_api.get_ip4h(); + ps.sip = ip4h->get_src(); + ps.dip = ip4h->get_dst();; ps.zero = 0; ps.protocol = IPPROTO_TCP; ps.len = htons((uint16_t)*len); @@ -709,8 +710,9 @@ bool TcpCodec::update(Packet* p, Layer* lyr, uint32_t* len) else { checksum::Pseudoheader6 ps6; - memcpy(ps6.sip, &p->ip_api.get_ip6_src()->u6_addr32, sizeof(ps6.sip)); - memcpy(ps6.dip, &p->ip_api.get_ip6_dst()->u6_addr32, sizeof(ps6.dip)); + const ipv6::IP6RawHdr* ip6h = p->ip_api.get_ip6h(); + memcpy(ps6.sip, ip6h->get_src()->u6_addr32, sizeof(ps6.sip)); + memcpy(ps6.dip, ip6h->get_dst()->u6_addr32, sizeof(ps6.dip)); ps6.zero = 0; ps6.protocol = IPPROTO_TCP; ps6.len = htons((uint16_t)*len); diff --git a/src/codecs/ip/cd_udp.cc b/src/codecs/ip/cd_udp.cc index e429d3dce..fb8a37bed 100644 --- a/src/codecs/ip/cd_udp.cc +++ b/src/codecs/ip/cd_udp.cc @@ -450,8 +450,8 @@ bool UdpCodec::update(Packet* p, Layer* lyr, uint32_t* len) if (p->ip_api.is_ip4()) { checksum::Pseudoheader ps; const ip::IPHdr* ip4h = p->ip_api.get_ip4h(); - ps.sip = ((IPHdr *)(lyr-1)->start)->ip_src; - ps.dip = ((IPHdr *)(lyr-1)->start)->ip_dst; + ps.sip = ip4h->get_src(); + ps.dip = ip4h->get_dst(); ps.zero = 0; ps.protocol = IPPROTO_UDP; ps.len = htons((uint16_t)*len); diff --git a/src/codecs/link/cd_pppencap.cc b/src/codecs/link/cd_pppencap.cc index 89c36c6c6..9870c32d2 100644 --- a/src/codecs/link/cd_pppencap.cc +++ b/src/codecs/link/cd_pppencap.cc @@ -140,7 +140,7 @@ bool PppEncap::decode(const uint8_t *raw_pkt, const uint32_t& raw_len, case PPP_VJ_UCOMP: /* VJ compression modifies the protocol field. It must be set * to tcp (only TCP packets can be VJ compressed) */ - if(raw_len < (lyr_len + ip::IP4_HEADER_LEN)) + if(raw_len < (uint32_t)(lyr_len + ip::IP4_HEADER_LEN)) { if (ScLogVerbose()) ErrorMessage("PPP VJ min packet length > captured len! " diff --git a/src/codecs/misc/CMakeLists.txt b/src/codecs/misc/CMakeLists.txt index f4ffb17b9..ac437522d 100644 --- a/src/codecs/misc/CMakeLists.txt +++ b/src/codecs/misc/CMakeLists.txt @@ -14,6 +14,9 @@ if(STATIC_CODECS) else(STATIC_CODECS) add_shared_library(cd_gtp codecs cd_gtp.cc cd_gtp_module.h cd_gtp_module.cc) add_shared_library(cd_teredo codecs cd_teredo.cc) + add_shared_library(cd_ip4_embedded_in_icmp codecs cd_ip4_embedded_in_icmp.cc) + add_shared_library(cd_ip6_embedded_in_icmp codecs cd_ip6_embedded_in_icmp.cc) + add_shared_library(cd_prot_embedded_in_icmp codecs cd_prot_embedded_in_icmp.cc) # When static codecs NOT enabled, the icmp files will be included in the # ICMP libraries in the other file. diff --git a/src/codecs/misc/Makefile.am b/src/codecs/misc/Makefile.am index 571fee91f..99dd3442c 100644 --- a/src/codecs/misc/Makefile.am +++ b/src/codecs/misc/Makefile.am @@ -4,13 +4,14 @@ noinst_LIBRARIES = libmisc_codecs.a libmisc_codecs_a_SOURCES = \ cd_default.cc - - plugin_list = \ cd_gtp.cc \ cd_gtp_module.h \ cd_gtp_module.cc \ -cd_teredo.cc +cd_teredo.cc \ +cd_ip4_embedded_in_icmp.cc \ +cd_ip6_embedded_in_icmp.cc \ +cd_prot_embedded_in_icmp.cc if STATIC_CODECS libmisc_codecs_a_SOURCES += $(plugin_list) @@ -28,6 +29,21 @@ ehlib_LTLIBRARIES += libcd_teredo.la libcd_teredo_la_CXXFLAGS = $(AM_CXXFLAGS) -DBUILDING_SO libcd_teredo_la_LDFLAGS = -export-dynamic -shared libcd_teredo_la_SOURCES = cd_teredo.cc + +ehlib_LTLIBRARIES += libcd_ip4_embedded_in_icmp.la +libcd_ip4_embedded_in_icmp_la_CXXFLAGS = $(AM_CXXFLAGS) -DBUILDING_SO +libcd_ip4_embedded_in_icmp_la_LDFLAGS = -export-dynamic -shared +libcd_ip4_embedded_in_icmp_la_SOURCES = cd_ip4_embedded_in_icmp.cc + +ehlib_LTLIBRARIES += libcd_ip6_embedded_in_icmp.la +libcd_ip6_embedded_in_icmp_la_CXXFLAGS = $(AM_CXXFLAGS) -DBUILDING_SO +libcd_ip6_embedded_in_icmp_la_LDFLAGS = -export-dynamic -shared +libcd_ip6_embedded_in_icmp_la_SOURCES = cd_ip6_embedded_in_icmp.cc + +ehlib_LTLIBRARIES += libcd_prot_embedded_in_icmp.la +libcd_prot_embedded_in_icmp_la_CXXFLAGS = $(AM_CXXFLAGS) -DBUILDING_SO +libcd_prot_embedded_in_icmp_la_LDFLAGS = -export-dynamic -shared +libcd_prot_embedded_in_icmp_la_SOURCES = cd_prot_embedded_in_icmp.cc endif AM_CXXFLAGS = @AM_CXXFLAGS@ diff --git a/src/codecs/misc/cd_ip4_embedded_in_icmp.cc b/src/codecs/misc/cd_ip4_embedded_in_icmp.cc index 47906181b..0c0e9485d 100644 --- a/src/codecs/misc/cd_ip4_embedded_in_icmp.cc +++ b/src/codecs/misc/cd_ip4_embedded_in_icmp.cc @@ -58,8 +58,6 @@ public: } // namespace -// TODO: delete -#include void Ip4EmbeddedInIcmpCodec::get_protocol_ids(std::vector& v) { @@ -172,6 +170,7 @@ bool Ip4EmbeddedInIcmpCodec::decode(const uint8_t *raw_pkt, const uint32_t& raw_ break; } + lyr_len = ip::IP4_HEADER_LEN; return true; } diff --git a/src/codecs/misc/cd_ip6_embedded_in_icmp.cc b/src/codecs/misc/cd_ip6_embedded_in_icmp.cc index 27cbc3442..59a3d74cc 100644 --- a/src/codecs/misc/cd_ip6_embedded_in_icmp.cc +++ b/src/codecs/misc/cd_ip6_embedded_in_icmp.cc @@ -56,9 +56,6 @@ public: } // namespace -// TODO: delete -#include - void Ip6EmbeddedInIcmpCodec::get_protocol_ids(std::vector& v) { v.push_back(IP_EMBEDDED_IN_ICMP6); @@ -77,7 +74,7 @@ bool Ip6EmbeddedInIcmpCodec::decode(const uint8_t *raw_pkt, const uint32_t& raw_ (unsigned long) raw_len);); /* do a little validation */ - if ( raw_len < ipv6::hdr_len() ) + if ( raw_len < ipv6::IP6_HEADER_LEN ) { DEBUG_WRAP(DebugMessage(DEBUG_DECODE, "ICMP6: IP short header (%d bytes)\n", raw_len);); @@ -102,11 +99,11 @@ bool Ip6EmbeddedInIcmpCodec::decode(const uint8_t *raw_pkt, const uint32_t& raw_ return false; } - if ( raw_len < ipv6::hdr_len() ) + if ( raw_len < ipv6::IP6_HEADER_LEN ) { DEBUG_WRAP(DebugMessage(DEBUG_DECODE, "ICMP6: IP6 len (%d bytes) < IP6 hdr len (%d bytes), packet discarded\n", - raw_len, ipv6::hdr_len());); + raw_len, ipv6::IP6_HEADER_LEN);); codec_events::decoder_event(p, DECODE_ICMP_ORIG_DGRAM_LT_ORIG_IP); @@ -119,7 +116,7 @@ bool Ip6EmbeddedInIcmpCodec::decode(const uint8_t *raw_pkt, const uint32_t& raw_ // XXX NOT YET IMPLEMENTED - fragments inside ICMP payload DEBUG_WRAP(DebugMessage(DEBUG_DECODE, "ICMP6 Unreachable IP6 header length: " - "%lu\n", (unsigned long)ipv6::hdr_len());); + "%lu\n", (unsigned long)ipv6::IP6_HEADER_LEN);); // since we know the protocol ID in this layer (and NOT the // next layer), set the correct protocol here. Normally, @@ -145,6 +142,7 @@ bool Ip6EmbeddedInIcmpCodec::decode(const uint8_t *raw_pkt, const uint32_t& raw_ break; } + lyr_len = ipv6::IP6_HEADER_LEN; return true; } diff --git a/src/detection/detect.cc b/src/detection/detect.cc index 6bb916d0b..4cdf0ed2b 100644 --- a/src/detection/detect.cc +++ b/src/detection/detect.cc @@ -323,7 +323,7 @@ int Detect(Packet * p) // layer!! bool proto_found = false; ip::IpApi tmp_api; - uint8_t curr_layer = p->num_layers - 1; + int8_t curr_layer = p->num_layers - 1; while (!proto_found && layer::set_inner_ip_api(p, tmp_api, curr_layer)) diff --git a/src/detection/fpdetect.cc b/src/detection/fpdetect.cc index 800e5387e..5a31be0b2 100644 --- a/src/detection/fpdetect.cc +++ b/src/detection/fpdetect.cc @@ -552,7 +552,7 @@ static int rule_tree_match( void * id, void *tree, int index, void * data, void if (eval_data.p->packet_flags & PKT_IP_RULE) { ip::IpApi tmp_api = eval_data.p->ip_api; - uint8_t curr_layer = eval_data.p->num_layers -1; + int8_t curr_layer = eval_data.p->num_layers - 1; if (layer::set_inner_ip_api(eval_data.p, eval_data.p->ip_api, @@ -952,7 +952,7 @@ static inline int fpEvalHeaderSW(PORT_GROUP *port_group, Packet *p, int start_state; const uint8_t *tmp_payload; ip::IpApi tmp_api; - uint8_t curr_ip_layer = 0; + int8_t curr_ip_layer = 0; bool repeat = false; uint16_t tmp_dsize; FastPatternConfig *fp = snort_conf->fast_pattern_config; @@ -1495,7 +1495,7 @@ int fpEvalPacket(Packet *p) p->data = (const uint8_t *)udph + udp::UDP_HEADER_LEN; ip::IpApi tmp_api; - uint8_t curr_layer = 0; + int8_t curr_layer = 0; layer::set_outer_ip_api(p, tmp_api, curr_layer); if (tmp_api.pay_len() > udp::UDP_HEADER_LEN) diff --git a/src/log/log_text.cc b/src/log/log_text.cc index 9078b9f02..6dd12f585 100644 --- a/src/log/log_text.cc +++ b/src/log/log_text.cc @@ -1696,7 +1696,7 @@ void LogIPPkt(TextLog* log, int type, Packet * p) // FIXIT --> log everything in order!! ip::IpApi tmp_api = p->ip_api; - uint8_t num_layer = 0; + int8_t num_layer = 0; bool first = true; while (layer::set_outer_ip_api(p, p->ip_api, num_layer) && diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc index 0124cf417..68cef6426 100644 --- a/src/network_inspectors/binder/binder.cc +++ b/src/network_inspectors/binder/binder.cc @@ -177,8 +177,9 @@ int Binder::check_rules(Flow* flow, Packet* p) Port port = (p->packet_flags & PKT_FROM_CLIENT) ? p->dp : p->sp; - if (p->proto_bits & PROTO_BIT__VLAN) - uint16_t vlan = vlan::vth_vlan(layer::get_vlan_layer(p)); + // FIXIT -- use vlan when searching bindings +// if (p->proto_bits & PROTO_BIT__VLAN) +// uint16_t vlan = vlan::vth_vlan(layer::get_vlan_layer(p)); for ( i = 0; i < sz; i++ ) { diff --git a/src/protocols/Makefile.am b/src/protocols/Makefile.am index 9991874a9..ce170a05f 100644 --- a/src/protocols/Makefile.am +++ b/src/protocols/Makefile.am @@ -11,7 +11,6 @@ eth.h \ icmp4.h \ icmp6.h \ ip.h \ -ip.cc \ ipv4.h \ ipv6.h \ gre.h \ @@ -27,7 +26,8 @@ vlan.h \ wlan.h libprotocols_a_SOURCES = \ -layer.cc +layer.cc \ +ip.cc AM_CXXFLAGS = @AM_CXXFLAGS@ diff --git a/src/protocols/ip.h b/src/protocols/ip.h index b0169885f..b631c7852 100644 --- a/src/protocols/ip.h +++ b/src/protocols/ip.h @@ -42,6 +42,9 @@ struct Packet; +// FIXIT : can I assume api si always valid? i.e. if not ip4, then ipv6? +// or if not ip4, also make sure its not ip6 + namespace ip { diff --git a/src/protocols/ipv6.h b/src/protocols/ipv6.h index eaed80f1c..f47f41d26 100644 --- a/src/protocols/ipv6.h +++ b/src/protocols/ipv6.h @@ -43,11 +43,11 @@ namespace ipv6 namespace detail { -constexpr uint8_t IP6_HEADER_LEN = 40; constexpr uint8_t IP6_MULTICAST = 0xFF; // first/most significant octet constexpr uint32_t MIN_EXT_LEN = 8; } // namespace +constexpr uint8_t IP6_HEADER_LEN = 40; @@ -169,11 +169,11 @@ struct IP6RawHdr { return (uint8_t)(ntohl(ip6_vtf) >> 28); } inline uint8_t get_hdr_len() const - { return (uint8_t) detail::IP6_HEADER_LEN; } + { return (uint8_t) IP6_HEADER_LEN; } // becaise Snort expects this in terms of 32 bit words. inline uint8_t get_hlen() const - { return detail::IP6_HEADER_LEN / 4; } + { return IP6_HEADER_LEN / 4; } inline MulticastScope get_dst_multicast_scope() const { return static_cast(ip6_dst.u6_addr8[1] & 0x0F); } @@ -212,12 +212,6 @@ struct IP6RawHdr }; - -inline uint8_t hdr_len() -{ - return detail::IP6_HEADER_LEN; -} - inline bool is_multicast(uint8_t addr) { return addr == detail::IP6_MULTICAST; diff --git a/src/protocols/layer.cc b/src/protocols/layer.cc index 7713912be..249cac013 100644 --- a/src/protocols/layer.cc +++ b/src/protocols/layer.cc @@ -202,9 +202,9 @@ int get_inner_ip_lyr(const Packet* const p) bool set_inner_ip_api(const Packet* const p, ip::IpApi& api, - uint8_t& curr_layer) + int8_t& curr_layer) { - if (curr_layer >= p->num_layers) + if (curr_layer < 0 || curr_layer >= p->num_layers) return false; do @@ -219,6 +219,7 @@ bool set_inner_ip_api(const Packet* const p, const ip::IPHdr* ip4h = reinterpret_cast(lyr->start); api.set(ip4h); + curr_layer--; return true; } case ETHERTYPE_IPV6: @@ -227,23 +228,24 @@ bool set_inner_ip_api(const Packet* const p, const ipv6::IP6RawHdr* ip6h = reinterpret_cast(lyr->start); api.set(ip6h); + curr_layer--; return true; } //default: // don't care about this layer if its not IP. } - } while (curr_layer-- != 0); + } while (--curr_layer >= 0); return false; } bool set_outer_ip_api(const Packet* const p, ip::IpApi& api, - uint8_t& curr_layer) + int8_t& curr_layer) { uint8_t num_layers = p->num_layers; - if (curr_layer >= num_layers) + if (curr_layer < 0 || curr_layer >= num_layers) return false; do @@ -258,6 +260,7 @@ bool set_outer_ip_api(const Packet* const p, const ip::IPHdr* ip4h = reinterpret_cast(lyr->start); api.set(ip4h); + curr_layer++; return true; } case ETHERTYPE_IPV6: @@ -266,6 +269,7 @@ bool set_outer_ip_api(const Packet* const p, const ipv6::IP6RawHdr* ip6h = reinterpret_cast(lyr->start); api.set(ip6h); + curr_layer++; return true; } //default: diff --git a/src/protocols/layer.h b/src/protocols/layer.h index 8ef08c628..f7f515da8 100644 --- a/src/protocols/layer.h +++ b/src/protocols/layer.h @@ -132,11 +132,13 @@ int get_inner_ip_lyr(const Packet* const p); * PARAMS: * Packet* = packet struct containing data * ip::Api = ip api to be set - * uint8_t curr_layer = the current, zero based layer from which to - * start searching outwards. This field will be - * set to the Ip Api's layer, zeo based layer. + * int8_t curr_layer = the current, zero based layer from which to + * start searching outwards. Afte the fucntions, + * This field will be set to the layer before + * the Ip Api. If no IP layer is found, + * it will be set to zero. * - * 0<= curr_layer < p->num_layers + * 0 <= curr_layer < p->num_layers * RETURNS: * true: if the api is set * false: if the api has NOT been set @@ -146,12 +148,12 @@ int get_inner_ip_lyr(const Packet* const p); * * NOTE: curr_layer is zero based. That means to get all of the ip * layers (starting from teh innermost layer), during the first call - * 'curr_layer == p->num_layers - 1'. + * 'curr_layer == p->num_layers'. * * NOTE: This functions is extremely useful in a loop * while (set_inner_ip_api(p, api, layer)) { ... } */ -bool set_inner_ip_api(const Packet* const, ip::IpApi&, uint8_t& curr_layer); +bool set_inner_ip_api(const Packet* const, ip::IpApi&, int8_t& curr_layer); /* * Identical to above function except will begin searching from the @@ -161,7 +163,7 @@ bool set_inner_ip_api(const Packet* const, ip::IpApi&, uint8_t& curr_layer); * layers (starting from the OUTERMOST layer), during the first call * 'curr_layer == 0'. */ -bool set_outer_ip_api(const Packet* const, ip::IpApi&, uint8_t& curr_layer); +bool set_outer_ip_api(const Packet* const, ip::IpApi&, int8_t& curr_layer); } // namespace layer diff --git a/src/sfip/sfip_t.h b/src/sfip/sfip_t.h index e3c8d5c0c..5d1ba1619 100644 --- a/src/sfip/sfip_t.h +++ b/src/sfip/sfip_t.h @@ -88,9 +88,9 @@ char *sfip_to_str(const sfip_t *ip); */ static inline int sfip_is_set(const sfip_t& ip); static inline int sfip_is_set(const sfip_t* const ip); -static inline bool sfip_equals(const sfip_t& lhs, const sfip_t& rhs); -static inline bool sfip_unset_equals(const sfip_t& lhs, const sfip_t& rhs); -static inline bool sfip_not_equals(const sfip_t& lhs, const sfip_t& rhs); +static inline bool sfip_equals(const sfip_t* const lhs, const sfip_t* const rhs); +static inline bool sfip_unset_equals(const sfip_t* const lhs, const sfip_t* const rhs); +static inline bool sfip_not_equals(const sfip_t* const lhs, const sfip_t* const rhs); static inline bool sfip_lesser(const sfip_t* const lhs, const sfip_t* const rhs); static inline bool sfip_greater(const sfip_t* const lhs, const sfip_t* const rhs); static inline void sfip_clear(sfip_t& x);