ipsec: Improve the handling of NAT-T keepalive messages

Type: improvement

Ethernet frames on the wire are a minimum of 64 bytes, so use the length in the UDP header to determine if the ESP payload is one bytes of the special SPI, rather than the buffer's size (which will include the ethernet header's padding).
In the case of drop advance the packet back to the IP header so the ipx-drop node sees a sane packet.

Signed-off-by: Neale Ranns <neale@graphiant.com>
Change-Id: Ic3b75487919f0c77507d6f725bd11202bc5afee8

2 files changed, 34 insertions, 9 deletions

diff --git a/src/vnet/ipsec/ipsec_tun_in.c b/src/vnet/ipsec/ipsec_tun_in.c
index 4f8af006d2b..c414be05f7e 100644
--- a/src/vnet/ipsec/ipsec_tun_in.c
+++ b/src/vnet/ipsec/ipsec_tun_in.c
@@ -186,18 +186,35 @@ ipsec_tun_protect_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	{
 	  ip60 = (ip6_header_t *) ip40;
 	  esp0 = (esp_header_t *) (ip60 + 1);
-	  hdr_sz0 = sizeof (ip6_header_t);
+	  buf_rewind0 = hdr_sz0 = sizeof (ip6_header_t);
 	}
       else
 	{
-	  /* NAT UDP port 4500 case, don't advance any more */
 	  if (ip40->protocol == IP_PROTOCOL_UDP)
 	    {
+	      /* NAT UDP port 4500 case, don't advance any more */
 	      esp0 =
 		(esp_header_t *) ((u8 *) ip40 + ip4_header_bytes (ip40) +
 				  sizeof (udp_header_t));
 	      hdr_sz0 = 0;
 	      buf_rewind0 = ip4_header_bytes (ip40) + sizeof (udp_header_t);
+
+	      const udp_header_t *udp0 =
+		(udp_header_t *) ((u8 *) ip40 + ip4_header_bytes (ip40));
+
+	      /* length 9 = sizeof(udp_header) + 1 byte of special SPI */
+	      if (clib_net_to_host_u16 (udp0->length) == 9 &&
+		  esp0->spi_bytes[0] == 0xff)
+		{
+		  b[0]->error =
+		    node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_NAT_KEEPALIVE];
+
+		  next[0] = VNET_DEVICE_INPUT_NEXT_IP4_DROP;
+		  len0 = 0;
+
+		  vlib_buffer_advance (b[0], -buf_rewind0);
+		  goto trace00;
+		}
 	    }
 	  else
 	    {
@@ -213,15 +230,11 @@ ipsec_tun_protect_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
       if (len0 < sizeof (esp_header_t))
 	{
-	  if (esp0->spi_bytes[0] == 0xff)
-	    b[0]->error =
-	      node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_NAT_KEEPALIVE];
-	  else
-	    b[0]->error =
-	      node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_TOO_SHORT];
+	  b[0]->error = node->errors[IPSEC_TUN_PROTECT_INPUT_ERROR_TOO_SHORT];
 
 	  next[0] = is_ip6 ? VNET_DEVICE_INPUT_NEXT_IP6_DROP :
 			     VNET_DEVICE_INPUT_NEXT_IP4_DROP;
+	  vlib_buffer_advance (b[0], -buf_rewind0);
 	  goto trace00;
 	}
 
diff --git a/test/template_ipsec.py b/test/template_ipsec.py
index bbfe776f5c2..725cec58846 100644
--- a/test/template_ipsec.py
+++ b/test/template_ipsec.py
@@ -1293,10 +1293,13 @@ class IpsecTun4(object):
         self.verify_counters4(p, count)
 
     def verify_keepalive(self, p):
+        # the sizeof Raw is calculated to pad to the minimum ehternet
+        # frame size of 64 btyes
         pkt = (Ether(src=self.tun_if.remote_mac, dst=self.tun_if.local_mac) /
                IP(src=p.remote_tun_if_host, dst=self.tun_if.local_ip4) /
                UDP(sport=333, dport=4500) /
-               Raw(b'\xff'))
+               Raw(b'\xff') /
+               Padding(0 * 21))
         self.send_and_assert_no_replies(self.tun_if, pkt*31)
         self.assert_error_counter_equal(
             '/err/%s/NAT Keepalive' % self.tun4_input_node, 31)
@@ -1309,6 +1312,15 @@ class IpsecTun4(object):
         self.assert_error_counter_equal(
             '/err/%s/Too Short' % self.tun4_input_node, 31)
 
+        pkt = (Ether(src=self.tun_if.remote_mac, dst=self.tun_if.local_mac) /
+               IP(src=p.remote_tun_if_host, dst=self.tun_if.local_ip4) /
+               UDP(sport=333, dport=4500) /
+               Raw(b'\xfe') /
+               Padding(0 * 21))
+        self.send_and_assert_no_replies(self.tun_if, pkt*31)
+        self.assert_error_counter_equal(
+            '/err/%s/Too Short' % self.tun4_input_node, 62)
+
 
 class IpsecTun4Tests(IpsecTun4):
     """ UT test methods for Tunnel v4 """