ip: fix ICMP inner payload parsing

Add a check so that ICMP type is verified to be an error before parsing
inner payload. If it's not an error, then the inner payload is not there.

Type: fix
Fixes: 46d0ff3945
Change-Id: I5c7d8ddacb347ec030784f349064e66d63cd525e
Signed-off-by: Klement Sekera <[email protected]>

diff --git a/src/plugins/nat/det44/det44.h b/src/plugins/nat/det44/det44.h
index e576bfb..683f554 100644 (file)
--- a/src/plugins/nat/det44/det44.h
+++ b/src/plugins/nat/det44/det44.h
@@ -38,7 +38,6 @@
 #include <vnet/ip/reass/ip4_sv_reass.h>
 
 #include <nat/lib/lib.h>
-#include <nat/lib/inlines.h>
 #include <nat/lib/ipfix_logging.h>
 #include <nat/lib/nat_proto.h>
 

diff --git a/src/plugins/nat/det44/det44_in2out.c b/src/plugins/nat/det44/det44_in2out.c
index 3f5e05a..39a9eca 100644 (file)
--- a/src/plugins/nat/det44/det44_in2out.c
+++ b/src/plugins/nat/det44/det44_in2out.c
@@ -21,6 +21,7 @@
 #include <vlib/vlib.h>
 #include <vnet/vnet.h>
 #include <vnet/ip/ip.h>
+#include <vnet/ip/ip4_to_ip6.h>
 #include <vnet/fib/ip4_fib.h>
 #include <vppinfra/error.h>
 #include <vppinfra/elog.h>
@@ -29,7 +30,6 @@
 #include <nat/det44/det44_inlines.h>
 
 #include <nat/lib/lib.h>
-#include <nat/lib/inlines.h>
 #include <nat/lib/nat_inlines.h>
 
 typedef enum

diff --git a/src/plugins/nat/det44/det44_out2in.c b/src/plugins/nat/det44/det44_out2in.c
index ab6acd4..dd89606 100644 (file)
--- a/src/plugins/nat/det44/det44_out2in.c
+++ b/src/plugins/nat/det44/det44_out2in.c
@@ -21,6 +21,7 @@
 #include <vlib/vlib.h>
 #include <vnet/vnet.h>
 #include <vnet/ip/ip.h>
+#include <vnet/ip/ip4_to_ip6.h>
 #include <vnet/fib/ip4_fib.h>
 #include <vppinfra/error.h>
 #include <vppinfra/elog.h>
@@ -29,7 +30,6 @@
 #include <nat/det44/det44_inlines.h>
 
 #include <nat/lib/lib.h>
-#include <nat/lib/inlines.h>
 #include <nat/lib/nat_inlines.h>
 
 typedef enum

diff --git a/src/plugins/nat/dslite/dslite.h b/src/plugins/nat/dslite/dslite.h
index f05670c..979afb4 100644 (file)
--- a/src/plugins/nat/dslite/dslite.h
+++ b/src/plugins/nat/dslite/dslite.h
@@ -22,7 +22,6 @@
 
 #include <nat/lib/lib.h>
 #include <nat/lib/alloc.h>
-#include <nat/lib/inlines.h>
 
 typedef struct
 {

diff --git a/src/plugins/nat/dslite/dslite_in2out.c b/src/plugins/nat/dslite/dslite_in2out.c
index 522c3cf..806969f 100644 (file)
--- a/src/plugins/nat/dslite/dslite_in2out.c
+++ b/src/plugins/nat/dslite/dslite_in2out.c
@@ -12,6 +12,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#include <vnet/ip/ip4_to_ip6.h>
 #include <nat/dslite/dslite.h>
 #include <nat/lib/nat_syslog.h>
 

diff --git a/src/plugins/nat/dslite/dslite_out2in.c b/src/plugins/nat/dslite/dslite_out2in.c
index 531bbb4..9ec48d4 100644 (file)
--- a/src/plugins/nat/dslite/dslite_out2in.c
+++ b/src/plugins/nat/dslite/dslite_out2in.c
@@ -12,6 +12,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#include <vnet/ip/ip4_to_ip6.h>
 #include <nat/dslite/dslite.h>
 
 typedef enum

diff --git a/src/plugins/nat/lib/inlines.h b/src/plugins/nat/lib/inlines.h
deleted file mode 100644 (file)
index 24e3ba8..0000000
--- a/src/plugins/nat/lib/inlines.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Copyright (c) 2020 Cisco and/or its affiliates.
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-/**
- * @brief Common NAT inline functions
- */
-#ifndef included_nat_inlines_h__
-#define included_nat_inlines_h__
-
-#include <vnet/ip/icmp46_packet.h>
-
-static_always_inline u64
-icmp_type_is_error_message (u8 icmp_type)
-{
-  int bmp = 0;
-  bmp |= 1 << ICMP4_destination_unreachable;
-  bmp |= 1 << ICMP4_time_exceeded;
-  bmp |= 1 << ICMP4_parameter_problem;
-  bmp |= 1 << ICMP4_source_quench;
-  bmp |= 1 << ICMP4_redirect;
-  bmp |= 1 << ICMP4_alternate_host_address;
-
-  return (1ULL << icmp_type) & bmp;
-}
-
-#endif /* included_nat_inlines_h__ */
-/*
- * fd.io coding-style-patch-verification: ON
- *
- * Local Variables:
- * eval: (c-set-style "gnu")
- * End:
- */

diff --git a/src/plugins/nat/lib/ipfix_logging.c b/src/plugins/nat/lib/ipfix_logging.c
index 593fa09..f569ccd 100644 (file)
--- a/src/plugins/nat/lib/ipfix_logging.c
+++ b/src/plugins/nat/lib/ipfix_logging.c
@@ -22,7 +22,6 @@
 #include <vlibmemory/api.h>
 #include <vppinfra/atomics.h>
 #include <nat/lib/ipfix_logging.h>
-#include <nat/lib/inlines.h>
 
 vlib_node_registration_t nat_ipfix_flush_node;
 nat_ipfix_logging_main_t nat_ipfix_logging_main;

diff --git a/src/plugins/nat/lib/nat_syslog.c b/src/plugins/nat/lib/nat_syslog.c
index 98777eb..93756a5 100644 (file)
--- a/src/plugins/nat/lib/nat_syslog.c
+++ b/src/plugins/nat/lib/nat_syslog.c
@@ -21,7 +21,6 @@
 #include <vnet/syslog/syslog.h>
 
 #include <nat/lib/nat_syslog.h>
-#include <nat/lib/inlines.h>
 
 #include <nat/lib/nat_syslog_constants.h>
 

diff --git a/src/plugins/nat/nat44-ed/nat44_ed.h b/src/plugins/nat/nat44-ed/nat44_ed.h
index 7065114..c3a959b 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed.h
+++ b/src/plugins/nat/nat44-ed/nat44_ed.h
@@ -31,7 +31,6 @@
 #include <vlibapi/api.h>
 
 #include <nat/lib/lib.h>
-#include <nat/lib/inlines.h>
 
 /* default number of worker handoff frame queue elements */
 #define NAT_FQ_NELTS_DEFAULT 64

diff --git a/src/plugins/nat/nat44-ed/nat44_ed_inlines.h b/src/plugins/nat/nat44-ed/nat44_ed_inlines.h
index 04e5236..8cd93f2 100644 (file)
--- a/src/plugins/nat/nat44-ed/nat44_ed_inlines.h
+++ b/src/plugins/nat/nat44-ed/nat44_ed_inlines.h
@@ -27,6 +27,7 @@
 #include <nat/lib/log.h>
 #include <nat/lib/ipfix_logging.h>
 #include <nat/nat44-ed/nat44_ed.h>
+#include <vnet/ip/ip4_to_ip6.h>
 
 always_inline void
 init_ed_k (clib_bihash_kv_16_8_t *kv, u32 l_addr, u16 l_port, u32 r_addr,

diff --git a/src/plugins/nat/nat44-ei/nat44_ei.c b/src/plugins/nat/nat44-ei/nat44_ei.c
index e16625a..d1959f7 100644 (file)
--- a/src/plugins/nat/nat44-ei/nat44_ei.c
+++ b/src/plugins/nat/nat44-ei/nat44_ei.c
@@ -21,6 +21,7 @@
 #include <vnet/vnet.h>
 #include <vnet/ip/ip.h>
 #include <vnet/ip/ip4.h>
+#include <vnet/ip/ip4_to_ip6.h>
 #include <vnet/ip/ip_table.h>
 #include <vnet/ip/reass/ip4_sv_reass.h>
 #include <vnet/fib/fib_table.h>

diff --git a/src/plugins/nat/nat44-ei/nat44_ei.h b/src/plugins/nat/nat44-ei/nat44_ei.h
index b4aa0f2..786fb0c 100644 (file)
--- a/src/plugins/nat/nat44-ei/nat44_ei.h
+++ b/src/plugins/nat/nat44-ei/nat44_ei.h
@@ -35,7 +35,6 @@
 #include <vppinfra/hash.h>
 
 #include <nat/lib/lib.h>
-#include <nat/lib/inlines.h>
 #include <nat/lib/nat_proto.h>
 
 /* default number of worker handoff frame queue elements */

diff --git a/src/plugins/nat/nat44-ei/nat44_ei_in2out.c b/src/plugins/nat/nat44-ei/nat44_ei_in2out.c
index 3b981d6..2fbf283 100644 (file)
--- a/src/plugins/nat/nat44-ei/nat44_ei_in2out.c
+++ b/src/plugins/nat/nat44-ei/nat44_ei_in2out.c
@@ -21,6 +21,7 @@
 
 #include <vnet/vnet.h>
 #include <vnet/ip/ip.h>
+#include <vnet/ip/ip4_to_ip6.h>
 #include <vnet/ethernet/ethernet.h>
 #include <vnet/udp/udp_local.h>
 #include <vnet/fib/ip4_fib.h>

diff --git a/src/plugins/nat/nat44-ei/nat44_ei_out2in.c b/src/plugins/nat/nat44-ei/nat44_ei_out2in.c
index 5d91cb0..805a696 100644 (file)
--- a/src/plugins/nat/nat44-ei/nat44_ei_out2in.c
+++ b/src/plugins/nat/nat44-ei/nat44_ei_out2in.c
@@ -21,6 +21,7 @@
 
 #include <vnet/vnet.h>
 #include <vnet/ip/ip.h>
+#include <vnet/ip/ip4_to_ip6.h>
 #include <vnet/ethernet/ethernet.h>
 #include <vnet/udp/udp_local.h>
 #include <vnet/fib/ip4_fib.h>

diff --git a/src/plugins/nat/nat64/nat64.c b/src/plugins/nat/nat64/nat64.c
index 950eea6..c59cfbb 100644 (file)
--- a/src/plugins/nat/nat64/nat64.c
+++ b/src/plugins/nat/nat64/nat64.c
@@ -15,6 +15,7 @@
 
 #include <vppinfra/crc32.h>
 #include <vnet/fib/ip4_fib.h>
+#include <vnet/ip/ip4_to_ip6.h>
 
 #include <vnet/ip/reass/ip4_sv_reass.h>
 #include <vnet/ip/reass/ip6_sv_reass.h>

diff --git a/src/plugins/nat/nat64/nat64.h b/src/plugins/nat/nat64/nat64.h
index 9eb8d91..2577880 100644 (file)
--- a/src/plugins/nat/nat64/nat64.h
+++ b/src/plugins/nat/nat64/nat64.h
@@ -30,7 +30,6 @@
 #include <vnet/ip/reass/ip4_sv_reass.h>
 
 #include <nat/lib/lib.h>
-#include <nat/lib/inlines.h>
 #include <nat/lib/nat_inlines.h>
 
 #include <nat/nat64/nat64_db.h>

diff --git a/src/plugins/nat/nat64/nat64_db.c b/src/plugins/nat/nat64/nat64_db.c
index e4e9feb..6ba77c5 100644 (file)
--- a/src/plugins/nat/nat64/nat64_db.c
+++ b/src/plugins/nat/nat64/nat64_db.c
@@ -16,7 +16,6 @@
 #include <vnet/fib/fib_table.h>
 #include <nat/lib/ipfix_logging.h>
 #include <nat/lib/nat_syslog.h>
-#include <nat/lib/inlines.h>
 #include <nat/nat64/nat64_db.h>
 
 int

diff --git a/src/vnet/ip/ip4_to_ip6.h b/src/vnet/ip/ip4_to_ip6.h
index d356fd5..3c14a59 100644 (file)
--- a/src/vnet/ip/ip4_to_ip6.h
+++ b/src/vnet/ip/ip4_to_ip6.h
@@ -37,6 +37,20 @@ static u8 icmp_to_icmp6_updater_pointer_table[] =
 
 #define frag_id_4to6(id) (id)
 
+always_inline u64
+icmp_type_is_error_message (u8 icmp_type)
+{
+  int bmp = 0;
+  bmp |= 1 << ICMP4_destination_unreachable;
+  bmp |= 1 << ICMP4_time_exceeded;
+  bmp |= 1 << ICMP4_parameter_problem;
+  bmp |= 1 << ICMP4_source_quench;
+  bmp |= 1 << ICMP4_redirect;
+  bmp |= 1 << ICMP4_alternate_host_address;
+
+  return (1ULL << icmp_type) & bmp;
+}
+
 /**
  * @brief Get TCP/UDP port number or ICMP id from IPv4 packet.
  *
@@ -70,9 +84,14 @@ ip4_get_port (ip4_header_t *ip, u8 sender)
        *  - outer ICMP header length (2*sizeof (icmp46_header_t))
        *  - inner IP header length
        *  - first 8 bytes of payload of original packet in case of ICMP error
+       *
+       * Also make sure we only attempt to parse payload as IP packet if it's
+       * an ICMP error.
        */
       else if (clib_net_to_host_u16 (ip->length) >=
-              2 * sizeof (ip4_header_t) + 2 * sizeof (icmp46_header_t) + 8)
+                2 * sizeof (ip4_header_t) + 2 * sizeof (icmp46_header_t) +
+                  8 &&
+              icmp_type_is_error_message (icmp->type))
        {
          ip = (ip4_header_t *) (icmp + 2);
          if (PREDICT_TRUE ((ip->protocol == IP_PROTOCOL_TCP) ||

diff --git a/src/vnet/ip/ip6_to_ip4.h b/src/vnet/ip/ip6_to_ip4.h
index ebabcd0..931d2da 100644 (file)
--- a/src/vnet/ip/ip6_to_ip4.h
+++ b/src/vnet/ip/ip6_to_ip4.h
@@ -168,7 +168,19 @@ ip6_get_port (vlib_main_t *vm, vlib_buffer_t *b, ip6_header_t *ip6,
          if (dst_port)
            *dst_port = ((u16 *) (icmp))[2];
        }
-      else if (clib_net_to_host_u16 (ip6->payload_length) >= 64)
+      /*
+       * if there is enough data and ICMP type indicates ICMP error, then parse
+       * inner packet
+       *
+       * ICMP6 errors are:
+       *   1 - destination_unreachable
+       *   2 - packet_too_big
+       *   3 - time_exceeded
+       *   4 - parameter_problem
+       */
+      else if (clib_net_to_host_u16 (ip6->payload_length) >= 64 &&
+              icmp->type >= ICMP6_destination_unreachable &&
+              icmp->type <= ICMP6_parameter_problem)
        {
          u16 ip6_pay_len;
          ip6_header_t *inner_ip6;