]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
teredo: be stricter on what to consider valid teredo
authorVictor Julien <victor@inliniac.net>
Wed, 19 Dec 2018 08:45:35 +0000 (09:45 +0100)
committerVictor Julien <victor@inliniac.net>
Wed, 19 Dec 2018 18:26:08 +0000 (19:26 +0100)
Invalid Teredo can lead to valid DNS traffic (or other UDP traffic)
being misdetected as Teredo. This leads to false negatives in the
UDP payload inspection.

Make the teredo code only consider a packet teredo if the encapsulated
data was decoded without any 'invalid' events being set.

Bug #2736.

src/decode-ipv6.c
src/decode-teredo.c
src/decode.c
src/decode.h

index a54e71b24d6230c90cbca89a3b11de322a43e36d..3f8a52124e5a9f4024685f984c27bbfce75aede4 100644 (file)
@@ -150,9 +150,9 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
     SCEnter();
 
     uint8_t *orig_pkt = pkt;
-    uint8_t nh = 0; /* careful, 0 is actually a real type */
+    uint8_t nh = IPV6_GET_NH(p); /* careful, 0 is actually a real type */
     uint16_t hdrextlen = 0;
-    uint16_t plen;
+    uint16_t plen = len;
     char dstopts = 0;
     char exthdr_fh_done = 0;
     int hh = 0;
@@ -160,18 +160,18 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
     int eh = 0;
     int ah = 0;
 
-    nh = IPV6_GET_NH(p);
-    plen = len;
-
     while(1)
     {
-        /* No upper layer, but we do have data. Suspicious. */
-        if (nh == IPPROTO_NONE && plen > 0) {
-            ENGINE_SET_EVENT(p, IPV6_DATA_AFTER_NONE_HEADER);
+        if (nh == IPPROTO_NONE) {
+            if (plen > 0) {
+                /* No upper layer, but we do have data. Suspicious. */
+                ENGINE_SET_EVENT(p, IPV6_DATA_AFTER_NONE_HEADER);
+            }
             SCReturn;
         }
 
         if (plen < 2) { /* minimal needed in a hdr */
+            ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
             SCReturn;
         }
 
@@ -204,7 +204,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                 SCLogDebug("hdrextlen %"PRIu8, hdrextlen);
 
                 if (hdrextlen > plen) {
-                    ENGINE_SET_EVENT(p, IPV6_TRUNC_EXTHDR);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
                     SCReturn;
                 }
 
@@ -243,7 +243,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                 IPV6_SET_L4PROTO(p,nh);
                 hdrextlen =  (*(pkt+1) + 1) << 3;
                 if (hdrextlen > plen) {
-                    ENGINE_SET_EVENT(p, IPV6_TRUNC_EXTHDR);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
                     SCReturn;
                 }
 
@@ -288,7 +288,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                 if (optslen > plen) {
                     /* since the packet is long enough (we checked
                      * plen against hdrlen, the optlen must be malformed. */
-                    ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
                     /* skip past this extension so we can continue parsing the rest
                      * of the packet */
                     nh = *pkt;
@@ -311,7 +311,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                     }
 
                     if (offset + 1 >= optslen) {
-                        ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
+                        ENGINE_SET_INVALID_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
                         break;
                     }
 
@@ -320,7 +320,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
 
                     /* see if the optlen from the packet fits the total optslen */
                     if ((offset + 1 + ip6_optlen) > optslen) {
-                        ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
+                        ENGINE_SET_INVALID_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
                         break;
                     }
 
@@ -339,7 +339,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                         ra->ip6ra_len  = ip6_optlen;
 
                         if (ip6_optlen < sizeof(ra->ip6ra_value)) {
-                            ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
+                            ENGINE_SET_INVALID_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
                             break;
                         }
 
@@ -355,7 +355,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                         jumbo->ip6j_len  = ip6_optlen;
 
                         if (ip6_optlen < sizeof(jumbo->ip6j_payload_len)) {
-                            ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
+                            ENGINE_SET_INVALID_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
                             break;
                         }
 
@@ -370,7 +370,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                         hao->ip6hao_len  = ip6_optlen;
 
                         if (ip6_optlen < sizeof(hao->ip6hao_hoa)) {
-                            ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
+                            ENGINE_SET_INVALID_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN);
                             break;
                         }
 
@@ -422,7 +422,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                 uint16_t prev_hdrextlen = hdrextlen;
                 hdrextlen = sizeof(IPV6FragHdr);
                 if (hdrextlen > plen) {
-                    ENGINE_SET_EVENT(p, IPV6_TRUNC_EXTHDR);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
                     SCReturn;
                 }
 
@@ -468,7 +468,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                 IPV6_SET_L4PROTO(p,nh);
                 hdrextlen = sizeof(IPV6EspHdr);
                 if (hdrextlen > plen) {
-                    ENGINE_SET_EVENT(p, IPV6_TRUNC_EXTHDR);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
                     SCReturn;
                 }
 
@@ -497,7 +497,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
                 SCLogDebug("hdrextlen %"PRIu8, hdrextlen);
 
                 if (hdrextlen > plen) {
-                    ENGINE_SET_EVENT(p, IPV6_TRUNC_EXTHDR);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
                     SCReturn;
                 }
 
@@ -538,7 +538,7 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
             case IPPROTO_SHIM6:
                 hdrextlen = 8 + (*(pkt+1) * 8);  /* 8 bytes + length in 8 octet units */
                 if (hdrextlen > plen) {
-                    ENGINE_SET_EVENT(p, IPV6_TRUNC_EXTHDR);
+                    ENGINE_SET_INVALID_EVENT(p, IPV6_TRUNC_EXTHDR);
                     SCReturn;
                 }
                 nh = *pkt;
index 6739c24b59a3ff45c393a2fc064d7f7aa298382f..887366a61c99aa60574bd2efd7b9065d6f435e0a 100644 (file)
@@ -96,20 +96,34 @@ int DecodeTeredo(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt,
     /* There is no specific field that we can check to prove that the packet
      * is a Teredo packet. We've zapped here all the possible Teredo header
      * and we should have an IPv6 packet at the start pointer.
-     * We then can only do two checks before sending the encapsulated packets
+     * We then can only do a few checks before sending the encapsulated packets
      * to decoding:
      *  - The packet has a protocol version which is IPv6.
      *  - The IPv6 length of the packet matches what remains in buffer.
+     *  - HLIM is 0. This would technically be valid, but still weird.
+     *  - NH 0 (HOP) and not enough data.
+     *
+     *  If all these conditions are met, the tunnel decoder will be called.
+     *  If the packet gets an invalid event set, it will still be rejected.
      */
     if (IP_GET_RAW_VER(start) == 6) {
         IPV6Hdr *thdr = (IPV6Hdr *)start;
+
+        /* ignore hoplimit 0 packets, most likely an artifact of bad detection */
+        if (IPV6_GET_RAW_HLIM(thdr) == 0)
+            return TM_ECODE_FAILED;
+
+        /* if nh is 0 (HOP) with little data we have a bogus packet */
+        if (IPV6_GET_RAW_NH(thdr) == 0 && IPV6_GET_RAW_PLEN(thdr) < 8)
+            return TM_ECODE_FAILED;
+
         if (len ==  IPV6_HEADER_LEN +
                 IPV6_GET_RAW_PLEN(thdr) + (start - pkt)) {
             if (pq != NULL) {
                 int blen = len - (start - pkt);
                 /* spawn off tunnel packet */
                 Packet *tp = PacketTunnelPktSetup(tv, dtv, p, start, blen,
-                                                  DECODE_TUNNEL_IPV6, pq);
+                                                  DECODE_TUNNEL_IPV6_TEREDO, pq);
                 if (tp != NULL) {
                     PKT_SET_SRC(tp, PKT_SRC_DECODER_TEREDO);
                     /* add the tp to the packet queue. */
index 8af438ad7c580ce6975ad2698c40ea34d0d2370b..20e7a2cf24cfafb05b3b46a5697109d5a5cee2de 100644 (file)
@@ -79,6 +79,7 @@ int DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         case DECODE_TUNNEL_IPV4:
             return DecodeIPV4(tv, dtv, p, pkt, len, pq);
         case DECODE_TUNNEL_IPV6:
+        case DECODE_TUNNEL_IPV6_TEREDO:
             return DecodeIPV6(tv, dtv, p, pkt, len, pq);
         case DECODE_TUNNEL_VLAN:
             return DecodeVLAN(tv, dtv, p, pkt, len, pq);
@@ -87,7 +88,7 @@ int DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         case DECODE_TUNNEL_ERSPAN:
             return DecodeERSPAN(tv, dtv, p, pkt, len, pq);
         default:
-            SCLogInfo("FIXME: DecodeTunnel: protocol %" PRIu32 " not supported.", proto);
+            SCLogDebug("FIXME: DecodeTunnel: protocol %" PRIu32 " not supported.", proto);
             break;
     }
     return TM_ECODE_OK;
@@ -303,8 +304,12 @@ Packet *PacketTunnelPktSetup(ThreadVars *tv, DecodeThreadVars *dtv, Packet *pare
     ret = DecodeTunnel(tv, dtv, p, GET_PKT_DATA(p),
                        GET_PKT_LEN(p), pq, proto);
 
-    if (unlikely(ret != TM_ECODE_OK)) {
-        /* Not a tunnel packet, just a pseudo packet */
+    if (unlikely(ret != TM_ECODE_OK) ||
+            (proto == DECODE_TUNNEL_IPV6_TEREDO && (p->flags & PKT_IS_INVALID)))
+    {
+        /* Not a (valid) tunnel packet */
+        SCLogDebug("tunnel packet is invalid");
+
         p->root = NULL;
         UNSET_TUNNEL_PKT(p);
         TmqhOutputPacketpool(tv, p);
index a19166c4f93119009f6f67bdff909a4d56244493..5931304b98a1e0907697e7f7df729aa29ee987cf 100644 (file)
@@ -905,6 +905,7 @@ enum DecodeTunnelProto {
     DECODE_TUNNEL_VLAN,
     DECODE_TUNNEL_IPV4,
     DECODE_TUNNEL_IPV6,
+    DECODE_TUNNEL_IPV6_TEREDO,  /**< separate protocol for stricter error handling */
     DECODE_TUNNEL_PPP,
 };